Skip to content

Guard HDGF PointerFactory offset/length uint32 narrowing#1076

Merged
pjfanning merged 3 commits into
apache:trunkfrom
metsw24-max:hdgf-pointerfactory-overflow-guard
May 15, 2026
Merged

Guard HDGF PointerFactory offset/length uint32 narrowing#1076
pjfanning merged 3 commits into
apache:trunkfrom
metsw24-max:hdgf-pointerfactory-overflow-guard

Conversation

@metsw24-max
Copy link
Copy Markdown
Contributor

Replace unchecked uint32-to-int narrowing in HDGF PointerFactory for offset and length fields with Math.toIntExact.

These values originate from LittleEndian.getUInt(...) and later flow into stream offset/length arithmetic and StreamStore construction. Malformed values above Integer.MAX_VALUE could previously wrap into negative or truncated integers.

This change aligns PointerFactory with the HDGF ChunkHeader hardening introduced in PR #1075 and with the broader parser hardening work using Math.toIntExact for structural size and offset fields.

Address parsing intentionally remains unchanged because the value is only used for diagnostic/debug output and does not participate in bounds calculations.

Added regression tests covering:

  • oversized offset rejection
  • oversized length rejection
  • v5 and v6+ parsing paths
  • acceptance of Integer.MAX_VALUE boundary values

// would silently wrap to a negative int rather than letting the wrapped
// value reach the downstream IOUtils.safelyClone bounds check.
p.setOffset(Math.toIntExact(LittleEndian.getUInt(data, offset+8)));
p.setLength(Math.toIntExact(LittleEndian.getUInt(data, offset+12)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any chance that you could get these values as longs and then call the check methods on IOUtils? At least for length. The exception messages thrown by IOUtils are more informative than 'int overflow'.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done both fields are now read as long up-front.
Length goes through IOUtils.safelyAllocateCheck(value, Integer.MAX_VALUE); Offset has a small helper throwing RecordFormatException with the offending value
Tests and poi-integration-exceptions.csv updated to match the new exception.

Copy link
Copy Markdown
Member

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - thanks

@pjfanning pjfanning merged commit bc6aa26 into apache:trunk May 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants