win: fix TOCTOU race and int truncation in make_program_env()

GetEnvironmentVariableW is called twice: first to measure the value
size, then to fetch it. If the variable grows between these two calls,
the second call writes past the buffer before the size mismatch check
at the TODO comment can fire, causing heap corruption.

Fix by:
- Capping the remaining buffer size to INT_MAX before the cast to int,
  preventing negative values from implicit signed-to-unsigned conversion
- Checking the return value of GetEnvironmentVariableW against the
  actual remaining buffer size instead of the stale measurement
- Gracefully skipping the variable if it was deleted or grew, instead
  of calling uv_fatal_error

This resolves the TODO at the existing 'handle race condition?' comment.
This commit is contained in:
Ali Raza 2026-03-07 03:09:20 +05:00
parent 64fbbe3888
commit 36364c3d74

View File

@ -757,13 +757,21 @@ int make_program_env(char* env_block[], WCHAR** dst_ptr) {
/* missing required var */
len = required_vars_value_len[i];
if (len) {
size_t remaining;
wcscpy(ptr, required_vars[i].wide_eq);
ptr += required_vars[i].len;
remaining = env_len - (ptr - dst);
if (remaining > (size_t) INT_MAX)
remaining = (size_t) INT_MAX;
var_size = GetEnvironmentVariableW(required_vars[i].wide,
ptr,
(int) (env_len - (ptr - dst)));
if (var_size != (DWORD) (len - 1)) { /* TODO: handle race condition? */
uv_fatal_error(GetLastError(), "GetEnvironmentVariableW");
(int) remaining);
if (var_size == 0 || var_size >= remaining) {
/* Env var was deleted or grew since we measured it; skip it. */
ptr -= required_vars[i].len;
len = 0;
} else {
len = var_size + 1;
}
}
i++;