Skip to content

HemfPlusPath: validate PointCount before array allocation#1103

Merged
pjfanning merged 2 commits into
apache:trunkfrom
jmestwa-coder:validate-path-pointcount
Jun 4, 2026
Merged

HemfPlusPath: validate PointCount before array allocation#1103
pjfanning merged 2 commits into
apache:trunkfrom
jmestwa-coder:validate-path-pointcount

Conversation

@jmestwa-coder
Copy link
Copy Markdown
Contributor

Summary

Validate PointCount before allocating path arrays in EmfPlusPath.

Problem

  • PointCount is read directly from EMF+ input.
  • The value is used as the length of both pathPoints and pointTypes arrays.
  • Invalid values can trigger allocation failures before any path data is processed.

Fix

  • Add validation for PointCount using IOUtils.safelyAllocateCheck(...).
  • Reject negative and oversized values before array allocation.
  • Align EmfPlusPath with the existing allocation-safety pattern used elsewhere in HEMF parsing.

* PointCount is an untrusted 32-bit field, so cap it before it is used as an array length -
* consistent with e.g. {@code HemfPlusDraw.MAX_OBJECT_SIZE} and {@code HemfDraw.MAX_NUMBER_OF_POLYGONS}.
*/
private static final int MAX_POINTS = 1_000_000;
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.

make this configurable by adding a static setter, it's ok to default to 1m

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.

Updated as suggested.

@jmestwa-coder jmestwa-coder force-pushed the validate-path-pointcount branch from 3ee351e to a61eaf7 Compare June 2, 2026 17:06
Comment thread poi-scratchpad/src/main/java/org/apache/poi/hemf/record/emfplus/HemfPlusPath.java Outdated
@jmestwa-coder jmestwa-coder force-pushed the validate-path-pointcount branch from a61eaf7 to f92d9fe Compare June 3, 2026 11:56
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

@pjfanning
Copy link
Copy Markdown
Member

@jmestwa-coder can you rebase this PR, there are merge conflicts

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.

needs rebase

@jmestwa-coder
Copy link
Copy Markdown
Contributor Author

rebase done.

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

@pjfanning pjfanning merged commit 103dd47 into apache:trunk Jun 4, 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