Skip to content

Commit b46ede7

Browse files
committed
(PUP-3837) FFI Registry access code
- Ruby 2.1.5 has introduced a number of bugs into the Windows Registry access code, including: - Forcing UTF-16LE read values to be re-encoded in the current local codepage, which can trigger encoding issues where valid Unicode characters in the registry cannot be translated properly (for instance en-dash U+2013 which cannot be represented in IBM437) These code paths may be triggered by the each_key or each_value enumerators which call an internal export_string method that performs this dangerous re-encoding. Also note that another helper called expand_environ may also trigger bad encoding conversions when accessing REG_MULTI_SZ value that have environment variables that must be expanded. Fortunately, these variables should more often than not be in the current active codepage and present less of a problem in actual use. - The inability to delete registry keys / values due to ANSI API declarations that are being fed wide character UTF-16LE strings https://bugs.ruby-lang.org/issues/10820 - Therefore, rewrite our own helpers around keys / values that convert UTF-16LE strings to UTF-8 as these are easier to work with in Ruby code. In practical terms, this conversion will be lossless as the only codepoints these encodings don't share are unused / invalid. Remove tests that were based on old ANSI API usage. Ensure Encoding.default_external is not used. - The impact of these changes is relatively low, and primarily affects the package provider and enumeration of installed Windows MSIs. Instead of using Win32::Registry#each_key, use Puppet::Util::Windows::Registry#each_key - Add FILETIME structure, LPBYTE and QWORD to FFI helpers - Add new integration test that demonstrates round-tripping a well known problematic Unicode string through the Registry that would trigger bad behavior in Win32::Registry. UTF-16LE bytes are written to the registry through Win32::Registry, read back through our helpers and compare to a well-known UTF-8 translation of said bytes. - Add Registry deletion helpers solely for the sake of working around Ruby's deletion bugs, so that integration test code can properly clean up after itself.
1 parent af02828 commit b46ede7

File tree

4 files changed

+341
-74
lines changed

4 files changed

+341
-74
lines changed

lib/puppet/provider/package/windows/package.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ def self.with_key(&block)
3535
mode |= KEY_READ
3636
begin
3737
open(hive, 'Software\Microsoft\Windows\CurrentVersion\Uninstall', mode) do |uninstall|
38-
uninstall.each_key do |name, wtime|
38+
each_key(uninstall) do |name, wtime|
3939
open(hive, "#{uninstall.keyname}\\#{name}", mode) do |key|
4040
yield key, values(key)
4141
end

lib/puppet/util/windows/api_types.rb

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ def read_win32_bool
4242

4343
alias_method :read_dword, :read_uint32
4444
alias_method :read_win32_ulong, :read_uint32
45+
alias_method :read_qword, :read_uint64
4546

4647
alias_method :read_hresult, :read_int32
4748

@@ -52,10 +53,10 @@ def read_handle
5253
alias_method :read_wchar, :read_uint16
5354
alias_method :read_word, :read_uint16
5455

55-
def read_wide_string(char_length)
56+
def read_wide_string(char_length, dst_encoding = Encoding.default_external)
5657
# char_length is number of wide chars (typically excluding NULLs), *not* bytes
5758
str = get_bytes(0, char_length * 2).force_encoding('UTF-16LE')
58-
str.encode(Encoding.default_external)
59+
str.encode(dst_encoding)
5960
end
6061

6162
def read_arbitrary_wide_string_up_to(max_char_length = 512)
@@ -139,6 +140,7 @@ def read_com_memory_pointer(&block)
139140
FFI.typedef :pointer, :lpcvoid
140141
FFI.typedef :pointer, :lpvoid
141142
FFI.typedef :pointer, :lpword
143+
FFI.typedef :pointer, :lpbyte
142144
FFI.typedef :pointer, :lpdword
143145
FFI.typedef :pointer, :pdword
144146
FFI.typedef :pointer, :phandle
@@ -229,6 +231,18 @@ def to_local_time
229231
end
230232
end
231233

234+
# https://msdn.microsoft.com/en-us/library/windows/desktop/ms724284(v=vs.85).aspx
235+
# Contains a 64-bit value representing the number of 100-nanosecond
236+
# intervals since January 1, 1601 (UTC).
237+
# typedef struct _FILETIME {
238+
# DWORD dwLowDateTime;
239+
# DWORD dwHighDateTime;
240+
# } FILETIME, *PFILETIME;
241+
class FILETIME < FFI::Struct
242+
layout :dwLowDateTime, :dword,
243+
:dwHighDateTime, :dword
244+
end
245+
232246
ffi_convention :stdcall
233247

234248
# http://msdn.microsoft.com/en-us/library/windows/desktop/aa366730(v=vs.85).aspx

lib/puppet/util/windows/registry.rb

Lines changed: 259 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ module Registry
1313
KEY_WRITE = 0x20006
1414
KEY_ALL_ACCESS = 0x2003f
1515

16+
ERROR_NO_MORE_ITEMS = 259
17+
1618
def root(name)
1719
Win32::Registry.const_get(name)
1820
rescue NameError
@@ -30,51 +32,277 @@ def open(name, path, mode = KEY_READ | KEY64, &block)
3032
end
3133
end
3234

33-
def values(subkey)
34-
values = {}
35-
subkey.each_value do |name, type, data|
35+
def keys(key)
36+
keys = {}
37+
each_key(key) { |subkey, filetime| keys[subkey] = filetime }
38+
keys
39+
end
40+
41+
# subkey is String which contains name of subkey.
42+
# wtime is last write time as FILETIME (64-bit integer). (see Registry.wtime2time)
43+
def each_key(key, &block)
44+
index = 0
45+
subkey = nil
46+
47+
begin
48+
subkey, filetime = reg_enum_key(key.hkey, index)
49+
yield subkey, filetime if !subkey.nil?
50+
index += 1
51+
end while !subkey.nil?
52+
53+
index
54+
end
55+
56+
def delete_key(key, subkey_name, mode = KEY64)
57+
reg_delete_key_ex(key.hkey, subkey_name, mode)
58+
end
59+
60+
def values(key)
61+
vals = {}
62+
each_value(key) { |subkey, type, data| vals[subkey] = data }
63+
vals
64+
end
65+
66+
def each_value(key, &block)
67+
index = 0
68+
subkey = nil
69+
70+
begin
71+
subkey, type, data = reg_enum_value(key.hkey, index)
72+
yield subkey, type, data if !subkey.nil?
73+
index += 1
74+
end while !subkey.nil?
75+
76+
index
77+
end
78+
79+
def delete_value(key, subkey_name)
80+
reg_delete_value(key.hkey, subkey_name)
81+
end
82+
83+
private
84+
85+
def reg_enum_key(hkey, index, max_key_length = Win32::Registry::Constants::MAX_KEY_LENGTH)
86+
subkey, filetime = nil, nil
87+
88+
FFI::MemoryPointer.new(:dword) do |subkey_length_ptr|
89+
FFI::MemoryPointer.new(FFI::WIN32::FILETIME.size) do |filetime_ptr|
90+
FFI::MemoryPointer.new(:wchar, max_key_length) do |subkey_ptr|
91+
subkey_length_ptr.write_dword(max_key_length)
92+
93+
# RegEnumKeyEx cannot be called twice to properly size the buffer
94+
result = RegEnumKeyExW(hkey, index,
95+
subkey_ptr, subkey_length_ptr,
96+
FFI::Pointer::NULL, FFI::Pointer::NULL,
97+
FFI::Pointer::NULL, filetime_ptr)
98+
99+
break if result == ERROR_NO_MORE_ITEMS
100+
101+
if result != FFI::ERROR_SUCCESS
102+
raise Puppet::Util::Windows::Error.new('Failed to enumerate registry keys')
103+
end
104+
105+
filetime = FFI::WIN32::FILETIME.new(filetime_ptr)
106+
subkey_length = subkey_length_ptr.read_dword
107+
subkey = subkey_ptr.read_wide_string(subkey_length, Encoding::UTF_8)
108+
end
109+
end
110+
end
111+
112+
[subkey, filetime]
113+
end
114+
115+
def reg_enum_value(hkey, index, max_value_length = Win32::Registry::Constants::MAX_VALUE_LENGTH)
116+
subkey, type, data = nil, nil, nil
117+
118+
FFI::MemoryPointer.new(:dword) do |subkey_length_ptr|
119+
FFI::MemoryPointer.new(:wchar, max_value_length) do |subkey_ptr|
120+
# RegEnumValueW cannot be called twice to properly size the buffer
121+
subkey_length_ptr.write_dword(max_value_length)
122+
123+
result = RegEnumValueW(hkey, index,
124+
subkey_ptr, subkey_length_ptr,
125+
FFI::Pointer::NULL, FFI::Pointer::NULL,
126+
FFI::Pointer::NULL, FFI::Pointer::NULL
127+
)
128+
129+
break if result == ERROR_NO_MORE_ITEMS
130+
131+
if result != FFI::ERROR_SUCCESS
132+
raise Puppet::Util::Windows::Error.new('Failed to enumerate registry values')
133+
end
134+
135+
subkey_length = subkey_length_ptr.read_dword
136+
subkey = subkey_ptr.read_wide_string(subkey_length, Encoding::UTF_8)
137+
138+
type, data = read(hkey, subkey_ptr)
139+
end
140+
end
141+
142+
[subkey, type, data]
143+
end
144+
145+
# Read a registry value named name and return array of
146+
# [ type, data ].
147+
# When name is nil, the `default' value is read.
148+
# type is value type. (see Win32::Registry::Constants module)
149+
# data is value data, its class is:
150+
# :REG_SZ, REG_EXPAND_SZ
151+
# String
152+
# :REG_MULTI_SZ
153+
# Array of String
154+
# :REG_DWORD, REG_DWORD_BIG_ENDIAN, REG_QWORD
155+
# Integer
156+
# :REG_BINARY
157+
# String (contains binary data)
158+
#
159+
# When rtype is specified, the value type must be included by
160+
# rtype array, or TypeError is raised.
161+
def read(hkey, name_ptr, *rtype)
162+
result = nil
163+
164+
query_value_ex(hkey, name_ptr) do |type, data_ptr, byte_length|
165+
unless rtype.empty? or rtype.include?(type)
166+
raise TypeError, "Type mismatch (expect #{rtype.inspect} but #{type} present)"
167+
end
168+
169+
string_length = 0
170+
# buffer is raw bytes, *not* chars - less a NULL terminator
171+
string_length = (byte_length / FFI.type_size(:wchar)) - 1 if byte_length > 0
172+
36173
case type
37-
when Win32::Registry::REG_MULTI_SZ
38-
data.each { |str| force_encoding(str) }
39174
when Win32::Registry::REG_SZ, Win32::Registry::REG_EXPAND_SZ
40-
force_encoding(data)
175+
result = [ type, data_ptr.read_wide_string(string_length, Encoding::UTF_8) ]
176+
when Win32::Registry::REG_MULTI_SZ
177+
result = [ type, data_ptr.read_wide_string(string_length, Encoding::UTF_8).split(/\0/) ]
178+
when Win32::Registry::REG_BINARY
179+
result = [ type, data.read_bytes(0, byte_length) ]
180+
when Win32::Registry::REG_DWORD
181+
result = [ type, data_ptr.read_dword ]
182+
when Win32::Registry::REG_DWORD_BIG_ENDIAN
183+
result = [ type, data_ptr.order(:big).read_dword ]
184+
when Win32::Registry::REG_QWORD
185+
result = [ type, data_ptr.read_qword ]
186+
else
187+
raise TypeError, "Type #{type} is not supported."
41188
end
42-
values[name] = data
43189
end
44-
values
190+
191+
result
45192
end
46193

47-
if defined?(Encoding)
48-
def force_encoding(str)
49-
if @encoding.nil?
50-
# See https://bugs.ruby-lang.org/issues/8943
51-
# Ruby uses ANSI versions of Win32 APIs to read values from the
52-
# registry. The encoding of these strings depends on the active
53-
# code page. However, ruby incorrectly sets the string
54-
# encoding to US-ASCII. So we must force the encoding to the
55-
# correct value.
56-
begin
57-
cp = GetACP()
58-
@encoding = Encoding.const_get("CP#{cp}")
59-
rescue
60-
@encoding = Encoding.default_external
194+
def query_value_ex(hkey, name_ptr, &block)
195+
FFI::MemoryPointer.new(:dword) do |type_ptr|
196+
FFI::MemoryPointer.new(:dword) do |length_ptr|
197+
result = RegQueryValueExW(hkey, name_ptr,
198+
FFI::Pointer::NULL, type_ptr,
199+
FFI::Pointer::NULL, length_ptr)
200+
201+
FFI::MemoryPointer.new(:byte, length_ptr.read_dword) do |buffer_ptr|
202+
result = RegQueryValueExW(hkey, name_ptr,
203+
FFI::Pointer::NULL, type_ptr,
204+
buffer_ptr, length_ptr)
205+
206+
if result != FFI::ERROR_SUCCESS
207+
raise Puppet::Util::Windows::Error.new("Failed to query registry value")
208+
end
209+
210+
# allows caller to use FFI MemoryPointer helpers to read / shape
211+
yield [type_ptr.read_dword, buffer_ptr, length_ptr.read_dword]
61212
end
62213
end
63-
64-
str.force_encoding(@encoding)
65214
end
66-
else
67-
def force_encoding(str, enc)
215+
end
216+
217+
def reg_delete_value(hkey, name)
218+
result = 0
219+
220+
FFI::Pointer.from_string_to_wide_string(name) do |name_ptr|
221+
result = RegDeleteValueW(hkey, name_ptr)
222+
223+
if result != FFI::ERROR_SUCCESS
224+
raise Puppet::Util::Windows::Error.new("Failed to delete registry value #{name}")
225+
end
68226
end
227+
228+
result
69229
end
70-
private :force_encoding
71230

231+
def reg_delete_key_ex(hkey, name, regsam = KEY64)
232+
result = 0
233+
234+
FFI::Pointer.from_string_to_wide_string(name) do |name_ptr|
235+
result = RegDeleteKeyExW(hkey, name_ptr, regsam, 0)
236+
237+
if result != FFI::ERROR_SUCCESS
238+
raise Puppet::Util::Windows::Error.new("Failed to delete registry key #{name}")
239+
end
240+
end
241+
242+
result
243+
end
72244

73245
ffi_convention :stdcall
74246

75-
# http://msdn.microsoft.com/en-us/library/windows/desktop/dd318070(v=vs.85).aspx
76-
# UINT GetACP(void);
77-
ffi_lib :kernel32
78-
attach_function_private :GetACP, [], :uint32
247+
# https://msdn.microsoft.com/en-us/library/windows/desktop/ms724862(v=vs.85).aspx
248+
# LONG WINAPI RegEnumKeyEx(
249+
# _In_ HKEY hKey,
250+
# _In_ DWORD dwIndex,
251+
# _Out_ LPTSTR lpName,
252+
# _Inout_ LPDWORD lpcName,
253+
# _Reserved_ LPDWORD lpReserved,
254+
# _Inout_ LPTSTR lpClass,
255+
# _Inout_opt_ LPDWORD lpcClass,
256+
# _Out_opt_ PFILETIME lpftLastWriteTime
257+
# );
258+
ffi_lib :advapi32
259+
attach_function_private :RegEnumKeyExW,
260+
[:handle, :dword, :lpwstr, :lpdword, :lpdword, :lpwstr, :lpdword, :pointer], :win32_long
261+
262+
# https://msdn.microsoft.com/en-us/library/windows/desktop/ms724865(v=vs.85).aspx
263+
# LONG WINAPI RegEnumValue(
264+
# _In_ HKEY hKey,
265+
# _In_ DWORD dwIndex,
266+
# _Out_ LPTSTR lpValueName,
267+
# _Inout_ LPDWORD lpcchValueName,
268+
# _Reserved_ LPDWORD lpReserved,
269+
# _Out_opt_ LPDWORD lpType,
270+
# _Out_opt_ LPBYTE lpData,
271+
# _Inout_opt_ LPDWORD lpcbData
272+
# );
273+
ffi_lib :advapi32
274+
attach_function_private :RegEnumValueW,
275+
[:handle, :dword, :lpwstr, :lpdword, :lpdword, :lpdword, :lpbyte, :lpdword], :win32_long
276+
277+
# https://msdn.microsoft.com/en-us/library/windows/desktop/ms724911(v=vs.85).aspx
278+
# LONG WINAPI RegQueryValueExW(
279+
# _In_ HKEY hKey,
280+
# _In_opt_ LPCTSTR lpValueName,
281+
# _Reserved_ LPDWORD lpReserved,
282+
# _Out_opt_ LPDWORD lpType,
283+
# _Out_opt_ LPBYTE lpData,
284+
# _Inout_opt_ LPDWORD lpcbData
285+
# );
286+
ffi_lib :advapi32
287+
attach_function_private :RegQueryValueExW,
288+
[:handle, :lpcwstr, :lpdword, :lpdword, :lpbyte, :lpdword], :win32_long
289+
290+
# LONG WINAPI RegDeleteValue(
291+
# _In_ HKEY hKey,
292+
# _In_opt_ LPCTSTR lpValueName
293+
# );
294+
ffi_lib :advapi32
295+
attach_function_private :RegDeleteValueW,
296+
[:handle, :lpcwstr], :win32_long
297+
298+
# LONG WINAPI RegDeleteKeyEx(
299+
# _In_ HKEY hKey,
300+
# _In_ LPCTSTR lpSubKey,
301+
# _In_ REGSAM samDesired,
302+
# _Reserved_ DWORD Reserved
303+
# );
304+
ffi_lib :advapi32
305+
attach_function_private :RegDeleteKeyExW,
306+
[:handle, :lpcwstr, :win32_ulong, :dword], :win32_long
79307
end
80308
end

0 commit comments

Comments
 (0)