Re: [ros-diffs] [weiden] 16553: handle memory allocation errors in GetEnvironmentVariable() and ensure the string is always null-terminated

[Joseph Galbraith]

> VarValue.MaximumLength = (nSize != 0 ? (nSize - 1) * sizeof(WCHAR) : 0);
>
> VarValue.Buffer = lpBuffer;
>
> Status = RtlQueryEnvironmentVariable_U (NULL,
>
> @@ -125,13 +150,17 @@
> SetLastErrorByStatus (Status);
> if (Status == STATUS_BUFFER_TOO_SMALL)
> {
>
> 	return (VarValue.Length / sizeof(WCHAR)) +  1;
>
> 	return (VarValue.Length / sizeof(WCHAR)) + 1;
>
> }
> else
> {
> 	return 0;
> }
> }
>
> /* make sure the string is NULL-terminated! RtlQueryEnvironmentVariable_U
>       only terminates it if MaximumLength < Length */
>
> VarValue.Buffer[VarValue.Length / sizeof(WCHAR)] = L'\0';

Is it possible for an environment variable's value to be empty?

If it is, RtlQueryEnvironmentVariable_U() could succeeded even
if a zero length buffer is passed in.  And then, would we crash
appending the NUL termination?

Since GetEnvironmentVariable has to have a buffer of at least
one character to succeed, maybe we should add such a guard to
the top of the function...

Thanks,

Joseph

[Thomas Weidenmueller]

Joseph Galbraith wrote:
> If it is, RtlQueryEnvironmentVariable_U() could succeeded even
> if a zero length buffer is passed in.  And then, would we crash
> appending the NUL termination?

That's true, i forgot to handle that case, it should be fixed in r16555.

Best Regards,
Thomas