Joseph Galbraith wrote:
If I read the code in RtlQueryEnvironmentVariable_U correctly (and I don't swear I have), it is possible for it to return STATUS_SUCCESS and not copy a NUL terminator. This happens if the buffer is large enough to contain the value but not the NUL terminator.
That is true and needs to be fixed.
But then, if I read the code in GetEnvironmentVariable() correctly, I don't believe it takes this situation into account.
Agreed, see above.
Probably, it would be best if RtlQueryEnvironmentVariable_U _never_ copied the NUL terminator, and the caller was always responsible for adding NUL termination, if needed.
No, it has to but it depends on whether there's enough space.
My proposal to fix the issue is that we change:
VarValue.MaximumLength = nSize * sizeof(WCHAR);
in GetEnvironmentVariableW to:
VarValue.MaximumLength = (nSize - 1) * sizeof(WCHAR);
Of course, similar changes would have to be applied to the ansi version. However, the case where nSize == 0 (and maybe also 1) would have to be handled appropriately. This way (given we set the termination character after the RtlQueryEnvironmentVariable_U call) we'd ensure the string is always NULL-terminated because RtlQueryEnvironmentVariable_U only terminates it if MaximumLength > Length.
Best Regards, Thomas