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 comparisons in str_squeeze. #4127

Merged
merged 1 commit into from Sep 25, 2018

Conversation

clayton-shopify
Copy link
Contributor

Dinko Galetic & Denis Kasak noticed that strings beginning with \xff can cause a crash in str_squeeze because s[0] is treated as a signed value, causing it to be equal to lastch, which is -1. After this occurs, i is decremented and s[-1] is accessed.

The following inputs demonstrate crashes:

"\xff00000000000000000000000".squeeze("\xff")
"\xff00000000000000000000000".squeeze!("\xff")
"\xff00000000000000000000000".squeeze
"\xff00000000000000000000000".squeeze!

Changing s to an unsigned char * fixes the problem.

@matz matz merged commit 152e8c5 into mruby:master Sep 25, 2018
@clayton-shopify clayton-shopify deleted the fix-str-squeeze-comparisons branch September 25, 2018 13:49
@matz
Copy link
Member

matz commented Sep 26, 2018

The issue is addressed by 9e3cbaa along with #4130. Reverted.

@dgaletic
Copy link

str_squeeze mishandles the ÿ character after the revert. It gets deleted if it's the first character in a string, e.g. p "ÿ".squeeze prints an empty string.

@matz
Copy link
Member

matz commented Sep 27, 2018

Ah, it doesn't work for ISO-8859-1 strings. Will be fixed soon.

matz added a commit that referenced this pull request Sep 27, 2018
ksekimoto added a commit to ksekimoto/mruby that referenced this pull request Jul 16, 2021
…mparisons

Fix comparisons in str_squeeze.
ksekimoto added a commit to ksekimoto/mruby that referenced this pull request Jul 16, 2021
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.

None yet

3 participants