Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

IonParser.getIntValue() fails or does not handle value overflow checks #428

Closed
cowtowncoder opened this issue Dec 20, 2023 · 1 comment
Closed

IonParser.getIntValue() fails or does not handle value overflow checks #428

cowtowncoder opened this issue Dec 20, 2023 · 1 comment
Labels
2.17 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue ion
Milestone
2.17.0

Comments

@cowtowncoder
Copy link
Member

cowtowncoder commented Dec 20, 2023 •
edited
Loading

(note: found via https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=65180)

Looks like overflow checks not being by IonParser can result in one of outcomes:

  • an Ion RuntimeException being thrown, when method getIntValue() is called with value beyond 32-bit int range
  • quiet truncation of value without failure

Instead, this should be surfaced same as equivalent JsonParseException JSON-backed JsonParser throws in similar situation -- see ParserBase.convertNumberToInt() (and ParserBase.reportOverflowInt()) for examples of handling.

Same probably affects long overflow as well (wrt BigInteger range value)
@cowtowncoder cowtowncoder added ion fuzz Issue found by OssFuzz 2.17 labels Dec 20, 2023
@cowtowncoder cowtowncoder changed the title Uncaught Ion exception for IonParser.getIntValue() with value overflow IonParser.getIntValue() fails or does not handle value overflow checks Dec 22, 2023
cowtowncoder added a commit that referenced this issue Dec 22, 2023
@cowtowncoder
Add failing tests for #428
3849e0f
cowtowncoder added a commit that referenced this issue Dec 22, 2023
@cowtowncoder
Add failing tests for #428 (#430)
ec572f1
@cowtowncoder
Copy link
Member Author

Since I do not have time to work on adding coercion checks quite yet, will just add failing tests for future work.
Should be relatively straight-forward for someone to pick up.

@cowtowncoder cowtowncoder added has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project and removed fuzz Issue found by OssFuzz labels Dec 22, 2023
thomasdelange5 pushed a commit to thomasdelange5/jackson-dataformats-binary that referenced this issue Feb 9, 2024
Add number size checks for IonParser getIntValue and getLongValue. Fi…
bd5479f 
…x failing tests for FasterXML#428
@thomasdelange5 thomasdelange5 mentioned this issue Feb 9, 2024
Merged
cowtowncoder pushed a commit that referenced this issue Feb 9, 2024
@thomasdelange5
Handle value overflow checks for IonParser.getIntValue() - #428 (#481)
cba329f
@cowtowncoder cowtowncoder removed the good first issue Issue that seems easy to resolve and is likely a good candidate for contributors new to project label Feb 12, 2024
@cowtowncoder cowtowncoder added this to the 2.17.0 milestone Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
2.17 has-failing-test Indicates that there exists a test case (under `failing/`) to reproduce the issue ion
Projects
None yet
Milestone
2.17.0
Development

No branches or pull requests
1 participant
@cowtowncoder