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.

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

Fix infinite loop bug #252

Merged
merged 5 commits into from
Aug 25, 2023
Merged

Conversation

arthurscchan
Copy link
Contributor

This fixes an infinite loop bug in src/main/java/org/fusesource/jansi/Ansi.java.

The scrollUp(int) and scrollDown(int) methods first check the input and determine what value to return. If a negative row number is provided, the method will use a minus sign to change it to a positive number and call its counterpart to handle. In general, this has no problem at all. But if the provided number is Integer.MIN_VALUE, which is -2147483648, it will create an infinite loop. The main reason is -(-2147483648) = 2147483648. But the max value of integer is only 2147483647 in java. Thus the number 2147483648 will wrap around and become -2147483648 again. This creates an infinite loop because the number always remains negative.

This PR fixes this possible bug by adding a conditional checking to ensure Integer.MIN_VALUE is changed to Integer.MAX_VALUE before calling the counterpart method to avoid the infinite loop.

We found this bug using fuzzing by way of OSS-Fuzz, where we recently integrated jansi (google/oss-fuzz#10705). OSS-Fuzz is a free service run by Google for fuzzing important open source software. If you'd like to know more about this then I'm happy to go in details and also set up things so you can receive emails and detailed reports when bugs are found.

Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
src/main/java/org/fusesource/jansi/Ansi.java Outdated Show resolved Hide resolved
src/main/java/org/fusesource/jansi/Ansi.java Outdated Show resolved Hide resolved
@gnodet
Copy link
Member

gnodet commented Aug 22, 2023

Thx, could you add an additional unit test ? that would be awesome @arthurscchan

Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
@arthurscchan
Copy link
Contributor Author

arthurscchan commented Aug 22, 2023

@gnodet Thanks for your reply. Yes I could add a unit test. Will do so soon.

Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
@gnodet gnodet added this to the 2.5.0 milestone Aug 25, 2023
Signed-off-by: Arthur Chan <arthur.chan@adalogics.com>
@gnodet gnodet merged commit c3ac54f into fusesource:master Aug 25, 2023
6 checks passed
@gnodet gnodet modified the milestones: 2.5.0, 2.4.1 Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants