jackson-core
jackson-core copied to clipboard
Fixes Implicit narrowing conversion in compound assignment FloatToDecimal
https://github.com/FasterXML/jackson-core/blob/6affde88dd3820b8c2eeb8700ae24b75651c8fcd/src/main/java/com/fasterxml/jackson/core/io/schubfach/FloatToDecimal.java#L408-L408 https://github.com/FasterXML/jackson-core/blob/6affde88dd3820b8c2eeb8700ae24b75651c8fcd/src/main/java/com/fasterxml/jackson/core/io/schubfach/FloatToDecimal.java#L415-L415 https://github.com/FasterXML/jackson-core/blob/6affde88dd3820b8c2eeb8700ae24b75651c8fcd/src/main/java/com/fasterxml/jackson/core/io/schubfach/FloatToDecimal.java#L426-L426 https://github.com/FasterXML/jackson-core/blob/6affde88dd3820b8c2eeb8700ae24b75651c8fcd/src/main/java/com/fasterxml/jackson/core/io/schubfach/FloatToDecimal.java#L440-L440
Fix the issue need to ensure that the type of the left-hand side (f) is at least as wide as the type of the right-hand side (pow10(H - len)). Since pow10 likely returns a long, the simplest and safest fix is to change the type of f from int to long. This avoids the implicit narrowing conversion and ensures that the computation can handle larger values without overflow.
Steps to implement the fix:
- Change the type of
ffrominttolongin the methodtoChars. - Update any other code within the method that interacts with
fto ensure compatibility with the new type. - Verify that the change does not introduce any unintended side effects or performance issues.
Compound assignment statements of the form x += y or x *= y perform an implicit narrowing conversion if the type of x is narrower than the type of y. x += y is equivalent to x = (T)(x + y), where T is the type of x. This can result in information loss and numeric errors such as overflows.
References
Compound Assignment Operators, Narrowing Primitive Conversion NUM00-J. Detect or prevent integer overflow
@odaysec can you provide a test case that demos that this is a fix? A test case that fails with the existing code and that is fixed by your change.
Yeah we definitely need something to show a problem to solve, not a speculative fix without one.
This has bit of a smell of tool/scanner reported concern.
Also worth noting that code for "schubfach" is an outside contribution so if there's an issue, should probably be fixed there (but I don't remember specifically where it's from, @pjfanning may know).
Yeah we definitely need something to show a problem to solve, not a speculative fix without one.
This has bit of a smell of tool/scanner reported concern.
Also worth noting that code for "schubfach" is an outside contribution so if there's an issue, should probably be fixed there (but I don't remember specifically where it's from, @pjfanning may know).
Comes from here: https://github.com/c4f7fcce9cb06515/Schubfach
No reproduction of an actual issue to fix, closing. May be re-opened/re-filed with some proof.