Skip to content

Fix problem with the negation in TextView/libswoc #10971

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.

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

Merged
merged 1 commit into from
Jan 14, 2024

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