Bug #11515: CreateProcessW() can cause "Invalid access to memory location"

• Status: Rejected
• Assignee: -
• Target version: -
• Backport: 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN

[docwhat (Christian Höltje)]

The second argument for CreateProcessW() needs to not be a constant because it may be modified:

From https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx :

The Unicode version of this function, CreateProcessW, can modify the contents of this string. Therefore, this parameter cannot be a pointer to read-only memory (such as a const variable or a literal string). If this parameter is a constant string, the function may cause an access violation.

This would explain sporadic "Invalid access to memory location" errors people see on Windows.

To resolve ths, cmd should be copied into a temporary variable before CreateProcessW() is called.

I've attached a patch that might work, but I'm not an expert C/Windows programmer.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Feedback

CreateChild is never called with a constant cmd, as it is build at the runtime.
How can you reproduce it?

[docwhat (Christian Höltje)]

Nobuyoshi Nakada wrote:

CreateChild is never called with a constant cmd, as it is build at the runtime.

Am I reading the code wrong then? The function declaration says prog is a const WCHAR *.

CreateChild(const WCHAR *cmd, const WCHAR *prog, ...)

As I said, I'm no expert at this. I googled a bunch and I found the MSDN page mentioned above. The only place it mentions memory access errors is if you pass lpCommandLine as a "constant string". I'm unsure if they mean the pointer to the string or, the memory the pointer points at, or both.

My patch assumes "both" and makes a copy.

How can you reproduce it?

Well, that's tricky since the OS decides whether or not to move things around. So there is no way to guarantee reproducing the error.

I should add, that I run across this regularly (several times a day) by running Chef on about 100 windows machine every 30 minutes. So... if you run it 300 times over a day on a windows machine you might reproduce it?

[docwhat (Christian Höltje)]

I found an alternative code version here: http://stackoverflow.com/questions/4514027/createprocessw-acess-violation-in-rtlinitunicodestring

//I'm copying the string here because CreateProcessW mutates its arguments
wchar_t *tmpCmd = _wcsdup(cmd);
...
free(tmpCmd);

As I said, I'm not huge C programmer and definitely not a windows programmer. I don't know what the difference between _wcsdup() vs. _tcscpy_s() are and why you'd want one over another. Though since _wcsdup() looks smarter since it'll allocate its own memory.

Someone asked what this meant here: https://social.msdn.microsoft.com/Forums/windowsdesktop/en-US/1e799be4-addf-40d8-b49a-cab2321accba/createprocessw-lpcommandline?forum=windowssdk -- I'm not sure if that helps, but I'm trying to do as suggested: create a WCHAR buffer to copy the strings into.

There is a blog post describing the history of CreateProcessW() which is informative: http://blogs.msdn.com/b/oldnewthing/archive/2009/06/01/9673254.aspx

As I said, I don't know enough. If the passed in cmd is not constant really and is acceptable, then ignore me.

[nobu (Nobuyoshi Nakada)]

• Status changed from Feedback to Rejected

cmd in CreateChild() is always allocated in heap.
win32/win32.c does not provide wchar-version spawn functions.