Security bug in ShellExecuteEx()

[Martin Fuchs]

On 09.04.2004 17:15:49 Francois Gouget wrote:
> Well, it's actually worst than that. Just before we have this code:
> 
>      if (wszCommandline[0]) {
>          strcatW(wszApplicationName, wSpace);
>          strcatW(wszApplicationName, wszCommandline);
>      }
> ...
>      /* Else, try to find the executable */
> 
> Now if you missed the beginning of ShellExecuteEx32() you would not 
> suspect this but wszApplicationName is the same as sei_tmp.lpFile. So 
> the above code, which was already there, would clobber sei_tmp.lpFile 
> whenever wszCommandline (i.e. in fact sei_tmp.lpParameters) is not 
> empty. This could probably cause ShellExecuteEx to fail in some 
> mysterious circumstances.

It wouldn't call it "clobber" - it's intended to change sei_tmp.lpFile 
I programmed it this way to pass modified versions of the
SHELLEXECUTEINFO structure to the worker functions.
This is used to expand environment strings and to translate
shell PIDLs to file system paths.

However all this shellexecute code is not trivial. There are really much
too many special cases to consider. If you ask me, it should be written
from scratch to clean up this mess - but who claims to be able to
do this?  ;-)

> ...
> So we end up with a wszApplicationName (i.e. an sei_tmp.lpFile) which is 
> quoted. Now as I have found out this does not work which is why I remove 
> the quotes... a bit before.
> So has this lpIDList code been tested? Does it work and if yes how come 
> ShellExecute("\"notepad.exe\"") didn't?

Well, I have tested it. But not directly in Wine. I am doing my shell32
tests with ReactOS Explorer using the MSVC debugger on Win XP. So may
be there are really quotation problems with this code in Wine.


Regards,

   Martin