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

[Frediano Ziglio]

> 
> 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
> ---
> Changes since v2: Fixed vcredist_x86.exe quoting, removed unneeded
> quoting
> 
>  win-guest-tools.nsis | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/win-guest-tools.nsis b/win-guest-tools.nsis
> index c54608a..19d45a3 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"

I tested this quoting with a small replacement executable.
The parameters should not be quoted.
Bad that installer silently did nothing and continued running.

> @@ -414,7 +414,7 @@ Function InstallService
>    ${endif}
>  
>    DetailPrint "Installing $R2 service"
> -  SimpleSC::InstallService $R0 $R2 16 2 $R1 "" "" ""
> +  SimpleSC::InstallService $R0 $R2 16 2 '"$R1"' "" "" ""
>    Pop $0
>    ${if} $0 != 0
>      DetailPrint "Failed to install $R2 service: $0"

Tested too.

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano