Uploaded image for project: 'Puppet'
  1. Puppet
  2. PUP-10254

Ensure Windows wide character strings have a wide terminator

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: PUP 5.5.21, PUP 6.17.0
    • Component/s: None
    • Labels:
    • Template:
      PUP Bug Template
    • Team:
      Coremunity
    • Sprint:
      Platform Core KANBAN
    • Method Found:
      Needs Assessment
    • Release Notes:
      Bug Fix
    • Release Notes Summary:
      Improves memory safety when puppet converts ruby strings to wide character strings on Windows.
    • QA Risk Assessment:
      Needs Assessment

      Description

      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.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              josh Josh Cooper
              Reporter:
              josh Josh Cooper
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Zendesk Support