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