Skip to content

Commit

Permalink
Windows - Disable option to add PATH during an AllUsers install (#584)
Browse files Browse the repository at this point in the history
* 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 <jaimergp@users.noreply.github.com>

* 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 <jaimergp@users.noreply.github.com>
  • Loading branch information
3 people authored Dec 13, 2022
1 parent 7307290 commit a1efe3b
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 27 deletions.
3 changes: 2 additions & 1 deletion CONSTRUCT.md
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,8 @@ _required:_ no<br/>
_type:_ boolean<br/>
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`

Expand Down
3 changes: 2 additions & 1 deletion constructor/construct.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, '''
Expand Down
25 changes: 12 additions & 13 deletions constructor/nsis/OptionsDialog.nsh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
53 changes: 41 additions & 12 deletions constructor/nsis/main.nsi.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand All @@ -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}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions news/584-windows-disable-path-for-allusers
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
### Enhancements

* <news item>

### 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

* <news item>

### Docs

* <news item>

### Other

0 comments on commit a1efe3b

Please sign in to comment.