Re: shell32: try to guess the working directory in the run dialog

[Juan Lang]

Hi Vincent,

+    while (*src)
+    {
+        *dest = *src;
+        if (*src == '\')
+        {
+            DWORD attrs = GetFileAttributesW(result);

It sure looks like you're calling GetFileAttributesW with a
non-NULL-terminated string.  I doubt this will work as you intend.
--Juan

shell32: try to guess the working directory in the run dialog

[Vincent Povirk]

I suppose it is possible for the string to not be null-terminated if
the command line ends in a backslash. I'll fix that and resend.

Vincent Povirk

On Tue, Oct 21, 2008 at 8:14 PM, Juan Lang  wrote:
> Hi Vincent,
> - while (*src)
> - {
> -
>    *dest = *src;
> -
>    if (*src == '\\')
> -
>    {
> -
>        DWORD attrs = GetFileAttributesW(result);
>
> It sure looks like you're calling GetFileAttributesW with a
> non-NULL-terminated string.  I doubt this will work as you intend.
> --Juan

shell32: try to guess the working directory in the run dialog

[Juan Lang]

> I suppose it is possible for the string to not be null-terminated if
> the command line ends in a backslash. I'll fix that and resend.

It's more probable than that, unless I'm missing something.  You
allocate result:
+    result = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
sizeof(WCHAR)*strlenW(cmdline));

set dest to result:
+    src = cmdline;
+    dest = result;

and copy src to dest, character by character:
+    while (*src)
+    {
+        *dest = *src;
where do you NULL-terminate dest?  Every backslash that's encountered
will result in GetFileAttributesW being called on a
non-NULL-terminated string, unless I'm missing something obvious.
--Juan

shell32: try to guess the working directory in the run dialog

[Juan Lang]

Oops, sorry Vincent, James caught me:
> - result = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
>
> sizeof(WCHAR)*strlenW(cmdline));

The HEAP_ZERO_MEMORY is the something obvious I was missing.

/me slinks back to his corner.
--Juan

shell32: try to guess the working directory in the run dialog

[Vincent Povirk]

Yep.

It's still broken though; the string I allocated is too small (strlen
excludes the null terminator), and that will be a problem if cmdline
ends in a backslash.

Vincent Povirk

On Tue, Oct 21, 2008 at 8:41 PM, Juan Lang  wrote:
> Oops, sorry Vincent, James caught me:
> > - result = HeapAlloc(GetProcessHeap(), HEAP_ZERO_MEMORY,
> >
> > sizeof(WCHAR)*strlenW(cmdline));
>
> The HEAP_ZERO_MEMORY is the something obvious I was missing.
>
> /me slinks back to his corner.
> --Juan

shell32: try to guess the working directory in the run dialog

[Francois Gouget]

On Tue, 21 Oct 2008, Vincent Povirk wrote:
> Yep.
>
> It's still broken though; the string I allocated is too small (strlen
> excludes the null terminator), and that will be a problem if cmdline
> ends in a backslash.

Also it would be nice to avoid the HEAP_ZERO_MEMORY if we are going to 
overwrite most of these zeroes anyway. I didn't really look at the code 
though.