Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix issues in GetEnvVarReliable #259

Merged
merged 1 commit into from
Apr 9, 2022
Merged

Conversation

mikeblas
Copy link
Contributor

GetEnvVarReliable() does some casting between size_t and DWORD, which is
problematic because size_t is eight bytes on Win32.

Further, the call to GetEnvVariable() may return non-zero but still not
copy any data to the output buffer. This case can be encountered when the
buffer is precisely the length of the required output, but can happen in
other circumstances. An extra check isn't expensive and makes this
code safer by correctly catching this error condition.

GetEnvVarReliable() does some casting between size_t and DWORD, which is
problematic because size_t is eight bytes on Win32.

Further, the call to GetEnvVariable() may return non-zero but still not
copy any data to the output buffer. This case can be encountered when the
buffer is precisely the length of the required output, but can happen in
other circumstances. An extra check isn't expensive and makes this
code safer by correctly catching this error condition.
@Lexikos Lexikos merged commit 2d23a0a into AutoHotkey:master Apr 9, 2022
@Lexikos
Copy link
Collaborator

Lexikos commented Apr 9, 2022

GetEnvVarReliable() does some casting between size_t and DWORD, which is
problematic because size_t is eight bytes on Win32.

I don't see what you're getting at. There was no casting, only implicit conversion from DWORD to size_t, which isn't inherently problematic.

Some static analysis tools will tell us that length + 1 might overflow before being assigned to a larger type, but the possibility of an environment variable of 0xfffffffe characters (producing a return value of 0xffffffff) is not a realistic concern.

This case can be encountered when the buffer is precisely the length of the required output

The buffer size is always 32767, which Microsoft states is the maximum size limit:

An environment variable has a maximum size limit of 32,767 characters, including the null-terminating character.

... but testing (on Windows 11) shows that environment variables can be larger than this.

GetEnvVarReliable is only used for A_ComSpec and the deprecated auto-env retrieval mechanism which is disabled by #NoEnv.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants