Skip to content

Commit aa4d21c

Browse files
g-bougardg-bougard
committed
fix: Fix regression from 1.7.2 which always set LOCAL if not used
CVE-2024-28241 fix is also optimized
1 parent 19e5e20 commit aa4d21c

File tree

4 files changed

+37
-24
lines changed

4 files changed

+37
-24
lines changed

‎Changes

+8
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ packaging:
7575
* Fix MSI to reuse InstallDir set in registry on upgrade
7676
* Updated Windows packages 7-Zip commandline tools to v23.01
7777

78+
1.7.3 not yet released
79+
80+
packaging:
81+
* Fix LOCAL is set to installation folder when LOCAL is not used on MSI windows
82+
installation, and even if it was set empty in installer UI
83+
* Enhanced CVE-2024-28241 fix to only apply folder security if folder is not a
84+
subfolder of system "Program Files" folder
85+
7886
1.7.2 Mon, 25 Mar 2024
7987

8088
packaging:

‎contrib/windows/glpi-agent-packaging.pl

+2-17
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use File::Spec;
88
use Cwd qw(abs_path);
99
use File::Spec::Functions qw(catfile);
10+
use Data::UUID;
1011

1112
use constant {
1213
PACKAGE_REVISION => "1", #BEWARE: always start with 1
@@ -90,6 +91,7 @@ sub build_app {
9091
agent_vertag => $versiontag // '',
9192
agent_fullname => $provider.' Agent',
9293
agent_rootdir => $provider.'-Agent',
94+
agent_localguid => Data::UUID->new()->create_str(),
9395
agent_regpath => "Software\\$provider-Agent",
9496
service_name => lc($provider).'-agent',
9597
msi_sharedir => 'contrib/windows/packaging',
@@ -490,23 +492,6 @@ sub _tree2xml {
490492
$result .= $ident ." ". qq[ <RemoveFolder Id="rm.$dir_id" On="uninstall" />\n];
491493
}
492494
$result .= $ident ." ". qq[</Component>\n];
493-
# Also add virtual folder properties under d_install
494-
if ($dir_id eq 'd_install') {
495-
foreach my $id (qw(LOCAL)) {
496-
$result .= $ident ." ". qq[<Directory Id="$id">\n];
497-
($component_id, $component_guid) = $self->_gen_component_id(lc($id).".create");
498-
$result .= $ident ." ". qq[<Component Id="$component_id" Guid="{$component_guid}" KeyPath="yes" Feature="$feat">\n];
499-
$result .= $ident ." ". qq[ <CreateFolder>\n];
500-
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="CREATOR OWNER" />\n];
501-
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="LocalSystem" />\n];
502-
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="Administrators" />\n];
503-
$result .= $ident ." ". qq[ <util:PermissionEx GenericWrite="no" GenericExecute="yes" GenericRead="yes" User="AuthenticatedUser" />\n];
504-
$result .= $ident ." ". qq[ </CreateFolder>\n];
505-
$result .= $ident ." ". qq[ <RemoveFolder Id="rm.] .lc($id). qq[" On="uninstall" />\n];
506-
$result .= $ident ." ". qq[</Component>\n];
507-
$result .= $ident ." ". qq[</Directory>\n];
508-
}
509-
}
510495
}
511496

512497
if (scalar(@f) > 0) {

‎contrib/windows/packaging/MSI_main-v2.wxs.tt

+26-7
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,9 @@
9797
<RegistrySearch Id="Local" Root="HKLM" Key="[%agent_regpath%]" Name="local" Type="raw"/>
9898
</Property>
9999
<SetProperty Id="CMDLINE_LOCAL" Before="AppSearch" Value="[LOCAL]" />
100-
<SetProperty Id="LOCAL" After="AppSearch" Value="[CMDLINE_LOCAL]"><![CDATA[CMDLINE_LOCAL<>"" OR CMDLINE_CONFIG="reset"]]></SetProperty>
100+
<!-- Also compare to ProgramFiles64Folder to fix wrongly set LOCAL in 1.7.2 -->
101+
<SetProperty Id="LOCAL" After="AppSearch" Value="[CMDLINE_LOCAL]"><![CDATA[CMDLINE_LOCAL<>"" OR LOCAL=INSTALLDIR OR CMDLINE_CONFIG="reset"]]></SetProperty>
102+
<SetDirectory Id="_LOCALDIR" Before="CostFinalize" Value="[LOCAL]" />
101103

102104
<Property Id="ADDITIONAL_CONTENT" Secure="yes">
103105
<RegistrySearch Id="AdditionalContent" Root="HKLM" Key="[%agent_regpath%]" Name="additional-content" Type="raw"/>
@@ -471,15 +473,19 @@
471473
<LaunchConditions Sequence="400" />
472474

473475
[%- IF bits==32 %]
474-
<Custom Action="SchedSecureObjects" After="CreateFolders"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
476+
<Custom Action="SchedSecureObjects" After="CreateFolders"><![CDATA[NOT INSTALLDIR<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
477+
<Custom Action="SetFixInstallDir" After="SchedSecureObjects"><![CDATA[NOT INSTALLDIR<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
478+
<Custom Action="FixInstallDir" After="SetFixInstallDir"><![CDATA[NOT INSTALLDIR<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
479+
<Custom Action="SetFixLocalDir" After="SchedSecureObjects"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
480+
<Custom Action="FixLocalDir" After="SetFixLocalDir"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
475481
[%- ELSE %]
476-
<Custom Action="SchedSecureObjects_x64" After="CreateFolders"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
482+
<Custom Action="SchedSecureObjects_x64" After="CreateFolders"><![CDATA[NOT INSTALLDIR<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
483+
<Custom Action="SetFixInstallDir" After="SchedSecureObjects_x64"><![CDATA[NOT INSTALLDIR<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
484+
<Custom Action="FixInstallDir" After="SetFixInstallDir"><![CDATA[NOT INSTALLDIR<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
485+
<Custom Action="SetFixLocalDir" After="SchedSecureObjects_x64"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
486+
<Custom Action="FixLocalDir" After="SetFixLocalDir"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
477487
[%- END %]
478-
<Custom Action="SetFixInstallDir" After="SchedSecureObjects"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
479-
<Custom Action="FixInstallDir" After="SetFixInstallDir"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
480488
<Custom Action="UpdateLocalDir" Before="CostFinalize"><![CDATA[LOCAL<>"" AND NOT LOCAL>>"\" AND NOT REMOVE~="ALL"]]></Custom>
481-
<Custom Action="SetFixLocalDir" After="SchedSecureObjects"><![CDATA[LOCAL<>"" AND NOT REMOVE~="ALL"]]></Custom>
482-
<Custom Action="FixLocalDir" After="SetFixLocalDir"><![CDATA[LOCAL<>"" AND NOT REMOVE~="ALL"]]></Custom>
483489

484490
<!-- Schedule custom action to always remove windows task if exists -->
485491
<Custom Action="SetEndTask" Before="EndTask" />
@@ -1037,6 +1043,19 @@
10371043
</Directory> <!-- INSTALLDIR -->
10381044
</Directory> <!-- ProgramFilesFolder -->
10391045

1046+
<Directory Id="_LOCALDIR">
1047+
<Component Id="LocalDir" Guid="$(var.LocalDirGuid)" KeyPath="yes" Feature="feat_AGENT">
1048+
<CreateFolder>
1049+
<util:PermissionEx GenericAll="yes" User="CREATOR OWNER" />
1050+
<util:PermissionEx GenericAll="yes" User="LocalSystem" />
1051+
<util:PermissionEx GenericAll="yes" User="Administrators" />
1052+
<util:PermissionEx GenericExecute="yes" GenericRead="yes" User="AuthenticatedUser" />
1053+
</CreateFolder>
1054+
<RemoveFolder Id="rm.local" On="uninstall" />
1055+
<Condition><![CDATA[LOCAL<>""]]></Condition>
1056+
</Component>
1057+
</Directory>
1058+
10401059
<Directory Id="ProgramMenuFolder" />
10411060

10421061
</Directory> <!-- TARGETDIR -->

‎contrib/windows/packaging/Variables-v2.wxi.tt

+1
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
<?define URLAbout = "[%url_about%]" ?> <!-- e.g. "https://glpi-project.org/" -->
1616
<?define URLHelp = "[%url_help%]" ?> <!-- e.g. "https://glpi-project.org/discussions/" -->
1717
<?define RootDir = "[%agent_rootdir%]" ?> <!-- e.g. "GLPI-Agent" -->
18+
<?define LocalDirGuid = "[%agent_localguid%]" ?>
1819

1920
<?define FileMainIcon = "[%msi_main_icon%]" ?>
2021
<?define FileLicenseRtf = "[%msi_license_rtf%]" ?>

0 commit comments

Comments
 (0)