Jira (PUP-10254) Don't rely on ruby wide string workaround when converting to wide strings

9 views
Skip to first unread message

Josh Cooper (JIRA)

unread,
Jan 25, 2020, 5:43:03 PM1/25/20
to puppe...@googlegroups.com
Josh Cooper created an issue
 
Puppet / Bug PUP-10254
Don't rely on ruby wide string workaround when converting to wide strings
Issue Type: Bug Bug
Assignee: Unassigned
Created: 2020/01/25 2:42 PM
Priority: Normal Normal
Reporter: Josh Cooper

Prior to ruby 2.1.0, it was possible for multibyte strings to have only a single null terminator, followed by garbage which could cause buffer overruns. This was fixed in https://github.com/ruby/ruby/commit/1549894a0699abdfab1fbba0e468afd3f2f990b3, but never backported to 1.9.3.

We added code to puppet to protect against this. See https://github.com/puppetlabs/puppet/commit/cabd95cbd06ac3d834e0e401bcb07c2878a0b0f2 and https://github.com/puppetlabs/puppet/commit/b7c10f5d045af5890f83159b74ab5d5a23e0abc9

However, puppet's from_string_to_wide_string copies the wide string from the ruby heap to the native heap using FFI::MemoryPointer#put_array_of_uchar. But it does not explicitly double null terminate the resulting wide string pointer. For example, here's a string of character length 1 and byte length 2.

irb(main):025:0> str = "a".encode("UTF-16LE")
=> "a"
irb(main):026:0> str.bytes.length
=> 2
irb(main):027:0> str.length
=> 1

When copying from the ruby heap to the native heap, we get a pointer to memory of length 2 bytes, which doesn't have room for the double null:

irb(main):028:0> ptr = FFI::MemoryPointer.new(:byte, str.bytes.length)
=> #<FFI::MemoryPointer address=0x0000000003aea340 size=2>
irb(main):029:0> ptr.total
=> 2

This hasn't been an issue because Puppet::Util::Windows::String.wide_string embeds a double null in the ruby string:

irb(main):030:0> str = Puppet::Util::Windows::String.wide_string("a")
=> "a\u0000"
irb(main):031:0> str.bytes.length
=> 4
irb(main):032:0> str.length
=> 2

We should always explicitly double null terminate the wide string in the native heap, so as to not rely on the ruby workaround.

Add Comment Add Comment
 
This message was sent by Atlassian JIRA (v7.7.1#77002-sha1:e75ca93)
Atlassian logo

Jorie Tappa (JIRA)

unread,
Jan 27, 2020, 12:58:04 PM1/27/20
to puppe...@googlegroups.com

Jorie Tappa (JIRA)

unread,
Jan 27, 2020, 12:59:04 PM1/27/20
to puppe...@googlegroups.com
Jorie Tappa commented on Bug PUP-10254
 
Re: Don't rely on ruby wide string workaround when converting to wide strings

We need to ensure other products we're using this same behavior in are also addressed

Josh Cooper (Jira)

unread,
Jun 18, 2020, 6:41:03 PM6/18/20
to puppe...@googlegroups.com
Josh Cooper assigned an issue to Josh Cooper
 
Change By: Josh Cooper
Assignee: Josh Cooper
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Josh Cooper (Jira)

unread,
Jun 18, 2020, 6:41:03 PM6/18/20
to puppe...@googlegroups.com
Josh Cooper updated an issue
Change By: Josh Cooper
Fix Version/s: PUP 6.17.0
Fix Version/s: PUP 5.5.21

Josh Cooper (Jira)

unread,
Jun 18, 2020, 6:41:04 PM6/18/20
to puppe...@googlegroups.com
Reply all
Reply to author
Forward
0 new messages