Skip to content

Commit

Permalink
fix(electron/windows): 🩹 prevent command LPE in the installer (#1703)
Browse files Browse the repository at this point in the history
This PR tries to prevent the local privilege escalation issue in Outline Client Windows installer by specifying the full path of system commands. Note that in a `.bat` file we can use `%SystemRoot%` variable (typically expands to "C:\Windows"), but it is not available in the `.nsh` NSIS script, therefore we need to use the [NSIS defined constant `$SYSDIR`](https://documentation.help/CTRE-NSIS/Section4.2.html#4.2.3) (typically expands to "C:\Windows\System32").

I also removed the Cygwin related information from the document because it is not required any more.

Fixes: b/266050099
  • Loading branch information
jyyi1 authored Aug 28, 2023
1 parent a1a9c30 commit ac4768e
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 21 deletions.
4 changes: 0 additions & 4 deletions src/electron/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ npm run action electron/start [windows|linux]

### Windows

Requirements for building on Windows:

- [Cygwin](https://cygwin.com/install.html), if running action scripts outside of `src`. It provides the "missing Unix pieces" required by build system such as rsync (and many others). Besides the default selected Unix tools such as `bash` and `rsync`, please also make sure to install `git` during Cygwin installation as well. You will need to clone this repository using `git` in Cygwin instead of the native Windows version of git, in order to ensure Unix line endings.

To build the _release_ version of Windows installer, you'll also need:

- [Java 8+ Runtime](https://www.java.com/en/download/). This is required for the cross-platform Windows executable signing tool [Jsign](https://ebourg.github.io/jsign/). If you don't need to sign the executables, feel free to skip this.
16 changes: 8 additions & 8 deletions src/electron/add_tap_device.bat
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ set ERROR_TAP_CONFIGURE_DNS=5
set PATH=%PATH%;%SystemRoot%\system32;%SystemRoot%\system32\wbem;%SystemRoot%\system32\WindowsPowerShell/v1.0

:: Check whether the device already exists.
netsh interface show interface name=%DEVICE_NAME%
%SystemRoot%\System32\netsh interface show interface name=%DEVICE_NAME%
if %errorlevel% equ 0 (
echo TAP network device already exists.
goto :configure
Expand Down Expand Up @@ -63,7 +63,7 @@ echo Found TAP device name: "%TAP_NAME%"
call :wait_for_device "%TAP_NAME%"

:: Attempt to rename the device even if waiting timed out.
netsh interface set interface name="%TAP_NAME%" newname="%DEVICE_NAME%"
%SystemRoot%\System32\netsh interface set interface name="%TAP_NAME%" newname="%DEVICE_NAME%"
if %errorlevel% neq 0 (
:: Try to rename the device through powershell in case netsh failed due to not being able to "see"
:: the device. Pipe input from /dev/null to prevent powershell from waiting forever on EOF.
Expand All @@ -89,7 +89,7 @@ call :wait_for_device "%DEVICE_NAME%"
::
:: So, continue even if this command fails - and always include its output.
echo (Re-)enabling TAP network device...
netsh interface set interface "%DEVICE_NAME%" admin=enabled
%SystemRoot%\System32\netsh interface set interface "%DEVICE_NAME%" admin=enabled
:: The powershell command is used to ensure the adapter is enabled if netsh fails and leaves it in
:: a disabled state. While no such failure has yet been observed, this command would correct it and
:: should behave idempotently otherwise.
Expand All @@ -102,7 +102,7 @@ powershell "Enable-NetAdapter -Name \"%DEVICE_NAME%\"" <nul
:: TODO: Actually search the system for an unused subnet or make the subnet
:: configurable in the Outline client.
echo Configuring TAP device subnet...
netsh interface ip set address %DEVICE_NAME% static 10.0.85.2 255.255.255.255
%SystemRoot%\System32\netsh interface ip set address %DEVICE_NAME% static 10.0.85.2 255.255.255.255
if %errorlevel% neq 0 (
echo Could not set TAP network device subnet. >&2
exit /b %ERROR_TAP_CONFIGURE_SUBNET%
Expand All @@ -114,13 +114,13 @@ if %errorlevel% neq 0 (
:: as it means we do not have to modify the DNS settings of any other network
:: device in the system. Configure with Cloudflare and Quad9 resolvers
echo Configuring primary DNS...
netsh interface ip set dnsservers %DEVICE_NAME% static address=1.1.1.1
%SystemRoot%\System32\netsh interface ip set dnsservers %DEVICE_NAME% static address=1.1.1.1
if %errorlevel% neq 0 (
echo Could not configure TAP device primary DNS. >&2
exit /b %ERROR_TAP_CONFIGURE_DNS%
)
echo Configuring secondary DNS...
netsh interface ip add dnsservers %DEVICE_NAME% 9.9.9.9 index=2
%SystemRoot%\System32\netsh interface ip add dnsservers %DEVICE_NAME% 9.9.9.9 index=2
if %errorlevel% neq 0 (
echo Could not configure TAP device secondary DNS. >&2
exit /b %ERROR_TAP_CONFIGURE_DNS%
Expand All @@ -132,14 +132,14 @@ exit /b 0
:: Exits with a non-zero code if the operation times out.
:wait_for_device
echo Testing that the network device "%~1" is visible to netsh...
netsh interface ip show interfaces | find "%~1" >nul 2>&1
%SystemRoot%\System32\netsh interface ip show interfaces | find "%~1" >nul 2>&1
if %errorlevel% equ 0 exit /b 0
for /l %%N in (1, 1, 6) do (
echo Waiting... %%N
:: timeout doesn't like the environment created by nsExec::ExecToStack and exits with:
:: "ERROR: Input redirection is not supported, exiting the process immediately."
waitfor /t 10 thisisnotarealsignalname >nul 2>&1
netsh interface ip show interfaces | find "%~1" >nul 2>&1
%SystemRoot%\System32\netsh interface ip show interfaces | find "%~1" >nul 2>&1
if !errorlevel! equ 0 exit /b 0
)
exit /b 1
10 changes: 5 additions & 5 deletions src/electron/custom_install_steps.nsh
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ ${StrRep}
File "${PROJECT_DIR}\src\electron\find_tap_device_name.bat"

; OutlineService files, stopping the service first in case it's still running.
nsExec::Exec "net stop OutlineService"
nsExec::Exec "$SYSDIR\net stop OutlineService"
File "${PROJECT_DIR}\tools\OutlineService\OutlineService\bin\OutlineService.exe"
File "${PROJECT_DIR}\tools\smartdnsblock\bin\smartdnsblock.exe"
File "${PROJECT_DIR}\third_party\newtonsoft\Newtonsoft.Json.dll"
Expand Down Expand Up @@ -147,7 +147,7 @@ ${StrRep}

nsExec::Exec install_windows_service.bat

nsExec::Exec "sc query OutlineService"
nsExec::Exec "$SYSDIR\sc query OutlineService"
Pop $0
StrCmp $0 0 success
; TODO: Trigger a Sentry report for service installation failure, too, and revisit
Expand All @@ -165,6 +165,6 @@ ${StrRep}
; with the bundled tapinstall.exe because it can only remove *all* devices
; having hwid tap0901 and these may include non-Outline devices.
!macro customUnInstall
nsExec::Exec "net stop OutlineService"
nsExec::Exec "sc delete OutlineService"
!macroend
nsExec::Exec "$SYSDIR\net stop OutlineService"
nsExec::Exec "$SYSDIR\sc delete OutlineService"
!macroend
8 changes: 4 additions & 4 deletions src/electron/install_windows_service.bat
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ setlocal EnableDelayedExpansion
set PWD=%~dp0%

:: Stop and delete the service.
net stop OutlineService
sc delete OutlineService
%SystemRoot%\System32\net stop OutlineService
%SystemRoot%\System32\sc delete OutlineService

:: Install and start the service, configuring it to restart on boot.
:: NOTE: spaces after the arguments are necessary for a correct installation, do not remove!
sc create OutlineService binpath= "%PWD%OutlineService.exe" displayname= "OutlineService" start= "auto"
net start OutlineService
%SystemRoot%\System32\sc create OutlineService binpath= "%PWD%OutlineService.exe" displayname= "OutlineService" start= "auto"
%SystemRoot%\System32\net start OutlineService

:: This is for the client: sudo-prompt discards stdout/stderr if the script
:: exits with a non-zero return code *which will happen if any of the previous
Expand Down

0 comments on commit ac4768e

Please sign in to comment.