Skip to content

Commit fd9ec39

Browse files
committed
(IAC-967) Append null terminators since it is no longer part of the data
Early ruby versions did not handle wide terminators correctly and would sometime read past the end of the string causing segfaults. To prevent that, puppet started embedding a wide terminator within the string. So the ruby string 'hi' would be encoded as the three character string 'hi\0', and then that would be encoded as UTF-16LE. While this works, it creates ambiguity as to whether the "\0" character is part of the data or not. For example, "hi\n\0".chop is "hi\n" whereas "hi\n".chop is "hi", and is probably what was intended. Ruby has since fixed its wide terminator handling, and as of Puppet 7, the `wide_string` method no longer embeds a terminator in the string. So update the registry module to explicitly add the terminator(s) before calling RegSetValueExW`.
1 parent ddb99e8 commit fd9ec39

File tree

1 file changed

+20
-4
lines changed

1 file changed

+20
-4
lines changed

lib/puppet/provider/registry_value/registry.rb

+20-4
Original file line numberDiff line numberDiff line change
@@ -178,16 +178,30 @@ def write_value
178178
raise error
179179
end
180180

181+
def wide_string_to_bytes(data)
182+
bytes = Puppet::Util::Windows::String.wide_string(data).bytes.to_a
183+
# versions prior to 7 embedded a wide null in the string content to work
184+
# around ruby bugs, see PUP-3970
185+
if Puppet::PUPPETVERSION[0].to_i >= 7
186+
bytes << 0 << 0
187+
else
188+
bytes
189+
end
190+
end
191+
192+
# This method must include wide null terminators in the returned
193+
# byte array for string-based registry values like REG_SZ. In
194+
# addition REG_MULTI_SZ must append another wide null character
195+
# to signify there are no more entries in the array.
181196
def data_to_bytes(type, data)
182197
bytes = []
183198

184199
case type
185200
when Win32::Registry::REG_SZ, Win32::Registry::REG_EXPAND_SZ
186-
bytes = Puppet::Util::Windows::String.wide_string(data).bytes.to_a
201+
bytes = wide_string_to_bytes(data)
187202
when Win32::Registry::REG_MULTI_SZ
188-
# each wide string is already NULL terminated
189-
bytes = data.map { |s| Puppet::Util::Windows::String.wide_string(s).bytes.to_a }.flat_map { |a| a }
190-
# requires an additional NULL terminator to terminate properly
203+
bytes = data.map { |s| wide_string_to_bytes(s) }.flat_map { |a| a }
204+
# requires an additional wide NULL terminator
191205
bytes << 0 << 0
192206
when Win32::Registry::REG_BINARY
193207
bytes = data.bytes.to_a
@@ -209,6 +223,8 @@ def write(reg, _name, type, data)
209223
bytes = data_to_bytes(type, data)
210224
FFI::MemoryPointer.new(:uchar, bytes.length) do |data_ptr|
211225
data_ptr.write_array_of_uchar(bytes)
226+
# From https://docs.microsoft.com/en-us/windows/win32/api/winreg/nf-winreg-regsetvalueexw
227+
# "cbData must include the size of the terminating null character or characters"
212228
if RegSetValueExW(reg.hkey, name_ptr, 0,
213229
type, data_ptr, data_ptr.size) != 0
214230
raise Puppet::Util::Windows::Error, 'Failed to write registry value'

0 commit comments

Comments
 (0)