From a1efe3b01bdbbcfde42d51bc6677cf23290ccb35 Mon Sep 17 00:00:00 2001 From: Paul Yim Date: Tue, 13 Dec 2022 10:40:27 -0500 Subject: [PATCH] Windows - Disable option to add PATH during an AllUsers install (#584) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Disable option to add installation to PATH during an AllUsers install * Add news file for release notes * Completely remove option to AddToPath when user selects AllUsers installation * Add note to help window, informing user the AddToPath is disable for AllUsers installs * Display MessageBox to alert user AddToPath is disabled for AllUsers * Move new item to Bug Fixes section * Update news/584-windows-disable-path-for-allusers Co-authored-by: Jaime Rodríguez-Guerra * Remove trailing slash * Indent and fix trailing slash * Add comment for why AddToPath is disabled for AllUsers; remove unnecessary block * When initialize_conda and initialize_by_default are True, only allow AddToPath if InstMode is JustMe * Remove second (unnecessary) call to insertmacro ParseCommandLineArgs * Add note to description for initialize_by_default * Fix line break docs linter error * Add note to CONSTRUCT.md docs page * Restore OnInit_Release function (can't confidently remove it at this point) * Relocate parsing of AddToPath option to OnInit_Release function Co-authored-by: Daniel Bast <2790401+dbast@users.noreply.github.com> Co-authored-by: Jaime Rodríguez-Guerra --- CONSTRUCT.md | 3 +- constructor/construct.py | 3 +- constructor/nsis/OptionsDialog.nsh | 25 +++++----- constructor/nsis/main.nsi.tmpl | 53 +++++++++++++++++----- news/584-windows-disable-path-for-allusers | 17 +++++++ 5 files changed, 74 insertions(+), 27 deletions(-) create mode 100644 news/584-windows-disable-path-for-allusers diff --git a/CONSTRUCT.md b/CONSTRUCT.md index 34b489a5a..ad287632a 100644 --- a/CONSTRUCT.md +++ b/CONSTRUCT.md @@ -524,7 +524,8 @@ _required:_ no
_type:_ boolean
Whether to add the installation to the PATH environment variable. The default is true for GUI installers (msi, pkg) and False for shell installers. The user -is able to change the default during interactive installation. +is able to change the default during interactive installation. NOTE: For Windows, +`AddToPath` is disabled when `InstallationType=AllUsers`. ## `register_python` diff --git a/constructor/construct.py b/constructor/construct.py index dcbd0a499..567a8704c 100644 --- a/constructor/construct.py +++ b/constructor/construct.py @@ -419,7 +419,8 @@ ('initialize_by_default', False, bool, ''' Whether to add the installation to the PATH environment variable. The default is true for GUI installers (msi, pkg) and False for shell installers. The user -is able to change the default during interactive installation. +is able to change the default during interactive installation. NOTE: For Windows, +`AddToPath` is disabled when `InstallationType=AllUsers`. '''), ('register_python', False, bool, ''' diff --git a/constructor/nsis/OptionsDialog.nsh b/constructor/nsis/OptionsDialog.nsh index 6fbc25e75..b1f975eeb 100644 --- a/constructor/nsis/OptionsDialog.nsh +++ b/constructor/nsis/OptionsDialog.nsh @@ -78,21 +78,20 @@ Function mui_AnaCustomOptions_Show ${NSD_OnClick} $mui_AnaCustomOptions.CreateShortcuts CreateShortcuts_OnClick ${If} "${SHOW_ADD_TO_PATH}" == "yes" + # AddToPath is only an option for JustMe installations; it is disabled for AllUsers + # installations. (Addresses CVE-2022-26526) ${If} $InstMode = ${JUST_ME} - StrCpy $1 "my" - ${Else} - StrCpy $1 "the system" + ${NSD_CreateCheckbox} 0 "$5u" 100% 11u "Add ${NAME} to my &PATH environment variable" + IntOp $5 $5 + 11 + Pop $mui_AnaCustomOptions.AddToPath + ${NSD_SetState} $mui_AnaCustomOptions.AddToPath $Ana_AddToPath_State + ${NSD_OnClick} $mui_AnaCustomOptions.AddToPath AddToPath_OnClick + ${NSD_CreateLabel} 5% "$5u" 90% 20u \ + "NOT recommended. This can lead to conflicts with other applications. Instead, use \ + the Commmand Prompt and Powershell menus added to the Windows Start Menu." + IntOp $5 $5 + 20 + Pop $Ana_AddToPath_Label ${EndIf} - ${NSD_CreateCheckbox} 0 "$5u" 100% 11u "Add ${NAME} to $1 &PATH environment variable" - IntOp $5 $5 + 11 - Pop $mui_AnaCustomOptions.AddToPath - ${NSD_SetState} $mui_AnaCustomOptions.AddToPath $Ana_AddToPath_State - ${NSD_OnClick} $mui_AnaCustomOptions.AddToPath AddToPath_OnClick - ${NSD_CreateLabel} 5% "$5u" 90% 20u \ - "NOT recommended. This can lead to conflicts with other applications. Instead, use \ - the Commmand Prompt and Powershell menus added to the Windows Start Menu." - IntOp $5 $5 + 20 - Pop $Ana_AddToPath_Label ${EndIf} ${If} "${SHOW_REGISTER_PYTHON}" == "yes" diff --git a/constructor/nsis/main.nsi.tmpl b/constructor/nsis/main.nsi.tmpl index dc8c1039a..1d87699dc 100644 --- a/constructor/nsis/main.nsi.tmpl +++ b/constructor/nsis/main.nsi.tmpl @@ -229,7 +229,10 @@ FunctionEnd Install for just me, add to PATH and register as system Python:$\n\ $EXEFILE /RegisterPython=1 /AddToPath=1$\n$\n\ Install for just me, with no registry modification (for CI):$\n\ - $EXEFILE /NoRegistry=1$\n" \ + $EXEFILE /NoRegistry=1$\n$\n\ + NOTE: If you install for AllUsers, then the option to AddToPath$\n\ + is disabled (i.e. if ./InstallationType=AllUsers, then$\n\ + /AddToPath=1 will be ignored).$\n" \ /SD IDOK Abort ${EndIf} @@ -254,16 +257,6 @@ FunctionEnd ${EndIf} ${EndIf} - ClearErrors - ${GetOptions} $ARGV "/AddToPath=" $ARGV_AddToPath - ${IfNot} ${Errors} - ${If} $ARGV_AddToPath = "1" - StrCpy $Ana_AddToPath_State ${BST_CHECKED} - ${ElseIf} $ARGV_AddToPath = "0" - StrCpy $Ana_AddToPath_State ${BST_UNCHECKED} - ${EndIf} - ${EndIf} - ClearErrors ${GetOptions} $ARGV "/KeepPkgCache=" $ARGV_KeepPkgCache ${If} ${Errors} @@ -312,6 +305,24 @@ Function OnInit_Release ${LogSet} on !insertmacro ParseCommandLineArgs + # Parsing the AddToPath option here (and not in ParseCommandLineArgs) to prevent the MessageBox from showing twice. + # For more context, see https://github.com/conda/constructor/pull/584#issuecomment-1347688020 + ClearErrors + ${GetOptions} $ARGV "/AddToPath=" $ARGV_AddToPath + ${IfNot} ${Errors} + ${If} $ARGV_AddToPath = "1" + ${If} $InstMode == ${ALL_USERS} + # To address CVE-2022-26526. + # In AllUsers install mode, do not allow AddToPath as an option. + MessageBox MB_OK|MB_ICONEXCLAMATION "/AddToPath=1 is disabled and ignored in 'All Users' installations" /SD IDOK + StrCpy $Ana_AddToPath_State ${BST_UNCHECKED} + ${Else} + StrCpy $Ana_AddToPath_State ${BST_CHECKED} + ${EndIf} + ${ElseIf} $ARGV_AddToPath = "0" + StrCpy $Ana_AddToPath_State ${BST_UNCHECKED} + ${EndIf} + ${EndIf} FunctionEnd Function InstModePage_RadioButton_OnClick @@ -599,7 +610,9 @@ Function .onInit # Override custom options with explicitly given values from contruct.yaml. # If initialize_by_default (register_python_default) is None, do nothing. #if initialize_conda is True and initialize_by_default is True - StrCpy $Ana_AddToPath_State ${BST_CHECKED} + ${If} $InstMode == ${JUST_ME} + StrCpy $Ana_AddToPath_State ${BST_CHECKED} + ${EndIF} #endif #if initialize_conda is True and initialize_by_default is False StrCpy $Ana_AddToPath_State ${BST_UNCHECKED} @@ -1103,6 +1116,22 @@ Section "Install" ${EndIf} WriteUninstaller "$INSTDIR\Uninstall-${NAME}.exe" + + # To address CVE-2022-26526. + # Revoke the write permission on directory "$INSTDIR" for Users if this is + # being run with administrative privileges. Users are: + # AU - authenticated users + # BU - built-in (local) users + # DU - domain users + ${If} ${UAC_IsAdmin} + DetailPrint "Setting installation directory permissions..." + AccessControl::DisableFileInheritance "$INSTDIR" + AccessControl::RevokeOnFile "$INSTDIR" "(AU)" "GenericWrite" + AccessControl::RevokeOnFile "$INSTDIR" "(DU)" "GenericWrite" + AccessControl::RevokeOnFile "$INSTDIR" "(BU)" "GenericWrite" + AccessControl::SetOnFile "$INSTDIR" "(BU)" "GenericRead + GenericExecute" + AccessControl::SetOnFile "$INSTDIR" "(DU)" "GenericRead + GenericExecute" + ${EndIf} SectionEnd !macro AbortRetryNSExecWaitLibNsisCmd cmd diff --git a/news/584-windows-disable-path-for-allusers b/news/584-windows-disable-path-for-allusers new file mode 100644 index 000000000..170afdbc2 --- /dev/null +++ b/news/584-windows-disable-path-for-allusers @@ -0,0 +1,17 @@ +### Enhancements + +* + +### Bug fixes + +* (For Windows only) Fix for [CVE-2022-26526](https://nvd.nist.gov/vuln/detail/CVE-2022-26526). Installations for "All Users" will not be allowed the option to modify the system PATH environment variable during installation. Installations for "Just Me" will still be allowed the option to add the installation to their PATH environment variable. Additionally, when installing with Administrator privileges, non-admin system Users will no longer have “Write” permissions. (#584) + +### Deprecations + +* + +### Docs + +* + +### Other