Win32SystemSupport>>setVariableWithRoot:path:key:value: is a method for
storing Strings in the Windows registry. It is based on wrong assumptions
which can lead to buffer overflows or similar errors (also in non-VW
applications).
To reproduce the bug, inspect the result of this snippet of code:
| interface value result |
interface := Win32SystemSupport new.
value := String new: 256 withAll: (Character value: 16rFFFF).
interface
setVariableWithPath: #('TEST')
key: 'aKey'
value: value.
result := interface
getVariableWithRoot: interface HKEY_CURRENT_USER
path: #('TEST')
name: 'aKey'
ifAbsent: []
The result will be a TwoByteString with more than 256 characters. The first
256 characters are 16rFFFF, which are followed by whatever comes next in
memory. If you are lucky and the first attempt yields a correct result
(which would be pure luck), repeat.
The method contains these lines:
--------
"We use asByteArrayEncoding: because encodeWide: would null-terminate
the string
and null termination is not needed here."
encodedValue := value isString
ifTrue: [value asByteArrayEncoding: self wideEncoding]
ifFalse: [value].
errorCode :=
self
RegSetValueEx: newKey contents asInteger
name: (self encodeWide: subkey)
reserved: 0
type: self REG_SZ
data: encodedValue
size: encodedValue size.
--------
The claim that null-termination is not needed for storing REG_SZ values is
plain wrong according to the MSDN documentation of the registry value types
at
http://msdn.microsoft.com/en-us/library/ms724884%28VS.85%29.aspxQuotes from MSDN:
"REG_SZ
A null-terminated string. This will be either a Unicode or an ANSI string,
depending on whether you use the Unicode or ANSI functions."
"When writing a string to the registry, you must specify the length of the
string, including the terminating null character (\0). A common error is to
use the strlen function to determine the length of the string, but to forget
that strlen returns only the number of characters in the string, not
including the terminating null."
When reading values which were stored without terminating nulls as in the VW
method, buffer overflows may be the result. Again from MSDN: "Therefore,
when reading a string from the registry, you must ensure that the string is
properly terminated before using it; otherwise, it may overwrite a buffer."
This is a risk for other applications which read the values stored by
VisualWorks.
The appended characters in the experiment above are caused by a second bug
in getVariableWithRoot:path:name:ifAbsent:.
The method does not use the size information from the registry to allocate a
buffer of an appropriate size, it uses the respective parameter of
RegQueryValueEx only as input for the function. Instead of reading the real
size of the value after the function call, it doubles the buffer size in a
loop until it is large enough. That's potentially inefficient. VW will
allocate a 32 MB buffer for all Strings with more than 8192 characters
(Unicode strings in the registry may contain 16,383 characters).
It also means that if the value stored in the registry does not have a
terminating null character, there will be more bytes copied into the result
string than necessary. When reading the buffer contents, it has to be
truncated to the number of characters answered by RegQueryValueEx.
This should be fixed asap.
Best regards,
Joachim Geidel
_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc
Locked
