pjfanning opened a new pull request, #1345: URL: https://github.com/apache/poi/pull/1345
Several parsers read an unsigned 32-bit count or size field, narrow it with `Math.toIntExact`, and only then range check it. For a field above `Integer.MAX_VALUE` the narrowing throws `ArithmeticException` first, so the check that follows never runs and the caller sees an unrelated exception instead of the `RecordFormatException` the guards are written to produce. This keeps those values as `long`s until after the check. The `IOUtils`/`ArrayUtil` checkers already take a `long` and already reject both negative and over-`Integer.MAX_VALUE` lengths with a descriptive message, so no new limit is introduced. ### Sites changed | Site | Guard that was pre-empted | |---|---| | `HemfDraw.EmfPolyBezier` / `EmfPolygon` | the 16K point cap | | `HemfDraw.EmfPolyDraw` | `HemfPicture.safelyAllocateCheck` | | `HemfComment.EmfCommentDataBeginGroup` / `EmfCommentDataWMF` | `IOUtils.safelyAllocate` | | `hpsf` `Section.readDictionary` | the `> 0xFFFFFF` oversize test | Two of these are behavioural rather than cosmetic: - **`EmfPolyBezier` / `EmfPolygon`** — the spec says extra points MUST be ignored, and the code caps the count at 16K to do exactly that. Because the narrowing ran first, a count above `Integer.MAX_VALUE` was rejected instead of clamped. - **`Section.readDictionary`** — the oversize test sits *outside* the `try`/`catch` that records a corrupted dictionary, so the narrowing threw `ArithmeticException` out of the `Section` constructor rather than taking the corrupted-dictionary path the guard exists to reach. ### Context The EMF records were reported by a security researcher as unbounded allocations. They are not: the allocation guards were added in #1104, #1107 and #1114, and all the reported sinks are already covered on trunk. `HemfRecordIterator` also wraps any `RuntimeException` into `RecordFormatException`, so the EMF paths are not an availability problem either. This PR only corrects the ordering so the existing guards report what they were meant to report. ### Tests Five regression tests, each verified to fail without the corresponding source change (`ArithmeticException` / wrong exception type) and pass with it: - `TestHemfPicture` — `testEmfPolyDrawCountAboveIntMax`, `testEmfPolygonCountAboveIntMaxIsClamped`, `testEmfCommentWmfSizeAboveIntMax`, `testEmfCommentBeginGroupDescriptionAboveIntMax` - `TestOverflowHardening` — `dictionaryEntryLengthAboveIntMaxIsTreatedAsCorrupt` 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
