Skip to content

Conversation

@shukitchan
Copy link
Contributor

@shukitchan shukitchan commented Jan 8, 2024

ABS_MIN is 9223372036854775808
intmax_t(ABS_MIN) is -9223372036854775808
negation of that is overflow for intmax_t data type with max value of 9223372036854775807
And I think that's not the intention as well.

So I need to make this change.

@shukitchan shukitchan requested a review from bneradt January 8, 2024 19:30
@shukitchan shukitchan self-assigned this Jan 8, 2024
@shukitchan shukitchan added this to the 10.0.0 milestone Jan 8, 2024
@shukitchan
Copy link
Contributor Author

This is needed for the problem found in oss fuzz - https://oss-fuzz.com/testcase-detail/5196561539530752

@shukitchan
Copy link
Contributor Author

/src/trafficserver/lib/swoc/src/TextView.cc:66:16: runtime error: negation of -9223372036854775808 cannot be represented in type 'intmax_t' (aka 'long'); cast to an unsigned type to negate this value to itself

@shukitchan shukitchan linked an issue Jan 8, 2024 that may be closed by this pull request
Copy link
Contributor

@bneradt bneradt left a comment

Choose a reason for hiding this comment

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

Ideally we would add a unit test for this, but I guess our copied libswoc doesn't have that.

Can you make a libswoc PR for this change and add a unit test here:
https://github.com/SolidWallOfCode/libswoc/blob/master/unit_tests/test_TextView.cc#L453

@bryancall bryancall requested a review from moonchen January 8, 2024 23:21
Copy link
Contributor

@moonchen moonchen left a comment

Choose a reason for hiding this comment

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

Agreed with @bneradt about the unit test. Otherwise, the code LGTM.

@shukitchan
Copy link
Contributor Author

Created SolidWallOfCode/libswoc#92

@shukitchan shukitchan merged commit dc4dc35 into apache:master Jan 14, 2024
phongn pushed a commit to phongn/trafficserver that referenced this pull request Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fuzzing: negation can result in overflow inside TextView::svtoi

4 participants