[Spice-devel] [nsis 1/2] Properly quote path to service binaries

[Christophe Fergeau]

On Sat, Dec 16, 2017 at 04:14:49AM -0500, Frediano Ziglio wrote:
> > 
> > If these paths are unquoted, and the path contains spaces (C:\Program
> > Files (x86)\...), this could be exploited by putting a binary with a
> > crafted name (C:\Program.exe), leading to privilege escalation as this
> > is a service that is being started.
> > 
> > https://www.commonexploits.com/unquoted-service-paths/
> > 
> > Bug reported by Chris Moberly
> 
> Yes, this is documented in CreateProcess and CreateService, specifically
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx
> and
> https://msdn.microsoft.com/en-us/library/windows/desktop/ms682450(v=vs.85).aspx
> 
> > ---
> >  win-guest-tools.nsis | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/win-guest-tools.nsis b/win-guest-tools.nsis
> > index c54608a..dfcee1d 100644
> > --- a/win-guest-tools.nsis
> > +++ b/win-guest-tools.nsis
> > @@ -114,7 +114,7 @@ Section "install"
> >    CreateDirectory "$INSTDIR\hooks\after_migration"
> >    CreateDirectory "$INSTDIR\hooks\before_migration"
> >  
> > -  ExecWait "$INSTDIR\vcredist_x86.exe /q"
> > +  ExecWait '"$INSTDIR\vcredist_x86.exe /q"'
> >  !endif
> >  
> >    SetOutPath "$INSTDIR\32"
> 
> No, you should not quote the argument together, should be
> 
>    ExecWait '"$INSTDIR\vcredist_x86.exe" /q'

Indeed, thanks.

> 
> > @@ -326,7 +326,7 @@ Function InstallDriver
> >    Pop $0
> >    StrCpy $1 $R1
> >    Push $1
> > -  StrCpy $2 "$INSTDIR\drivers\$0"
> > +  StrCpy $2 '"$INSTDIR\drivers\$0"'
> >    Push $2
> >    StrCpy $3 "$2\$R1.inf"
> >    Push $3
> 
> These are used to call SetupCopyOEMInf (https://msdn.microsoft.com/en-us/library/windows/desktop/aa376990(v=vs.85).aspx)
> which does not need quoting or are used with right quoting so quoting twice
> is not correct.

This did not seem to make a difference in my testing, so I kept it, but
I agree for driver installation it's probably less of an issue.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20171219/fd5da172/attachment.sig>