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

`sprintf` adds extra buffer slots when precision arg truncates string slices #6137

Closed
Magisus opened this issue Mar 24, 2020 · 2 comments
Closed

Comments

@Magisus
Copy link

@Magisus Magisus commented Mar 24, 2020

Environment Information

  • JRuby version: 9.2.7.0+
  • Operating system and platform: All

Expected Behavior

The sprintf function should correctly truncate string slices when a precision is specified that is smaller than the slice length.

Actual Behavior
The sprintf function misbehaves when using a precision number with a string argument, if that string is a slice of a larger string. It will add the wrong number of spaces to the buffer and then fill it with extra characters from the larger string, or leave the buffer spaces uninitialized if there are too few characters in the source string to fill it.

Script to repro:

long_string = "aabbccddhelloddccbbaa"
start_index = 8
sub_str_length = 5
precision = 3
sub_string = long_string[start_index, sub_str_length]
puts sprintf("%.#{precision}s", sub_string)
# => helloddccbb
# When calculating the precision, it adds (start_index (8) + precision (3) = 11) slots to the output buffer,
# then fills it with that many characters from the original larger string.
# If there are not enough characters to fill the number of slots that were added,
# it will output gibberish in the remaining slots (I assume these are just uninitialized bits)

I was able to fix this bug by adding this simple patch at https://github.com/jruby/jruby/blob/master/core/src/main/java/org/jruby/util/Sprintf.java#L622:

  len = StringSupport.nth(enc, bytes.getUnsafeBytes(), bytes.begin(), bytes.begin() + bytes.getRealSize(), precision);
+ len = len - bytes.begin(); // subtract the starting index to get the number of bytes to add
@headius

This comment has been minimized.

Copy link
Member

@headius headius commented Mar 24, 2020

cc @kares since I think the original commit was your work.

We do not really consider this a general security issue, but given the impact to Puppet we will put out a 9.2.11.1 release this week.

@headius headius added this to the 9.2.11.1 milestone Mar 24, 2020
headius added a commit to headius/jruby that referenced this issue Mar 24, 2020
This gets a bit deep into how string buffers are used and shared
but since it was exposed as a bug (jruby#6137) and CRuby
does the same copy-on-write buffer treatment that JRuby does, it
seemed an appropriate spec to add.
@headius headius linked a pull request that will close this issue Mar 24, 2020
@headius headius closed this in 626c098 Mar 25, 2020
@kares

This comment has been minimized.

Copy link
Member

@kares kares commented Mar 25, 2020

for the record, anyone reaching here this only concerns: %s with precision e.g. %.1s
bb90d3b introduced the issue (while attempting to fix multi-byte precision in %s).

@enebo enebo modified the milestones: 9.2.11.1, JRuby 9.2.11.1 Mar 25, 2020
eregon added a commit to ruby/spec that referenced this issue Mar 27, 2020
This gets a bit deep into how string buffers are used and shared
but since it was exposed as a bug (jruby/jruby#6137) and CRuby
does the same copy-on-write buffer treatment that JRuby does, it
seemed an appropriate spec to add.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

4 participants
You can’t perform that action at this time.