[323]

I thought it was parent's code.

Source seems to take into account the buffer overflow:

    DWORD length = required_length + 2;  // The +2 is for the maybe optional zero later on. Probably we are over-allocating.

[malf]

Does it? If required_length is 10, it allocates 12 bytes, and writes element 13, which is byte 26 and 27.

[lkey]

[in, out, optional] lpcbData (aka &length)

A pointer to a variable that specifies the size of the buffer pointed to by the lpData parameter, in bytes. When the function returns, this variable contains the size of the data copied to lpData.

In other words, it's rewritten to be the actual length before it's incremented for null termination.

[seritools]

`RegQueryValueExW`'s last parameter is in and out, so that `length` is set to the actual written length after the call.

It might cause an OOB write though, with a data race on the registry key (if the key's value happens to grow in length by a char or two between the calls, time of check time of use yada yada).

[323]

> It might cause an OOB write though

No, because `RegQueryValueExW` will return ERROR_MORE_DATA and the code bails out on error (also leaking the memory).

[seritools]

1. first call to `RegQueryValueExW` returns a value length of 10

2. length is set to 12

3. external change causes the value to now be 12

4. second call to ``RegQueryValueExW` succeeds, as 12 <= 12, no ERROR_MORE_DATA here; length stays 12

5. length + 1 and length + 2 are now OOB

[malft]

'by a char or two', they said.

[ectopod]

There are two off-by-one errors. Should be checking value[length-1].