Share via

Strings read from the registry are not guaranteed to be NULL terminated

  • Article

When reading from a string from the registry, most code I have reviewed assumes that it is NULL terminated and can use C runtime string functions like wcslen or wcscat on the buffer. Assuming the string is NULL terminated is a very bad idea and can lead to a buffer overflow (which others like Michael Howard have explained very well). A buffer overflow in a driver is about the worst spot for such an error since you can potentially gain complete control of the entire OS from within the kernel.

Instead of treating the buffer as a NULL terminated C-style string, you should always treat the buffer as counted string. Luckily, WDM has a counted string already, UNICODE_STRING. One gotcha that you need to watch out when converting the buffer to a UNICODE_STRING is that its length may contain the terminating NULL character. In this case, you want to change UNICODE_STRING::Length so that it does not include the NULL, otherwise the terminating NULL will be treated as a part of the buffer itself (vs a terminating NULL which is not considered a part of the name, just a signal that the buffer has ended)

Here is an example of how to initialize a UNICODE_STRING with a buffer read from the registry. Note that it does not make a copy of the value read from the registry, so you may need to modify the code to allocate a buffer for UNICODE_STRING::Buffer outside of the allocation for pPartial.

PKEY_VALUE_PARTIAL_INFORMATION pPartial;

 

[...open the key, read the length, allocate pPartial, read the buffer...]

 

if (pPartial->Type == REG_SZ &&

pPartial->DataLength <= USHORT_MAX) {

UNICODE_STRING tmp;

 

if (pPartial->DataLength == 0x0) {

//

// Empty string

//

tmp.Buffer = L"";

tmp.Length = 0;

tmp.MaximumLength = 0;

}

else {

 

//

// The string we read may not be NULL terminated, so put it into a

// UNICODE_STRING. If the final character is NULL, shorten the

// length of the string so that it does not include for the NULL.

//

// If there are embedded NULLs in the string previous to the final

// character, we leave them in place.

//

tmp.Buffer = (PWCH) &pPartial->Data[0];

tmp.Length = (USHORT) pPartial->DataLength; // cast is safe, we checked the range previously

tmp.MaximumLength = tmp.Length;

 

if (tmp.Buffer[(tmp.Length/sizeof(WCHAR))-1] == UNICODE_NULL) {

//

// Do not include the UNICODE_NULL in the length

//

tmp.Length -= sizeof(WCHAR);

}

 

[tmp is now properly initialized as a UNICODE_STRING representing the buffer read from the registry]

}

Comments

  • Anonymous
    December 06, 2006
    A lil late as a comment, I know, but this code should also probably check for a length that is not an even multiple of sizeof(WCHAR).  Depending on how code uses UNICODE_STRING structures and uses the Length member, an odd length might contribute to an off-by-one type problem with one byte of a WCHAR that is garbage (or possibly even resulting in a crash, if it happened to straddle a page boundary).

  • Anonymous
    December 13, 2006
    good point kevin, that part didn't occur to me.  I will have to go back and look at the KMDF code to see if we handle this case properly. d