Skip to content

Commit

Permalink
fix: Fix regression from 1.7.2 which always set LOCAL if not used
Browse files Browse the repository at this point in the history 
CVE-2024-28241 fix is also optimazed
  • Loading branch information
@g-bougard
g-bougard committed Mar 29, 2024
1 parent 6d050ba commit 7d37cf6
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 31 deletions.
8 changes: 8 additions & 0 deletions Changes
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
Revision history for GLPI agent

1.7.3 not yet released

packaging:
* Fix LOCAL is set to installation folder when LOCAL is not used on MSI windows
installation, and even if it was set empty in installer UI
* Enhanced CVE-2024-28241 fix to only apply folder security if folder is not a
subfolder of system "Program Files" folder

1.7.2 Mon, 25 Mar 2024

packaging:
Expand Down
33 changes: 9 additions & 24 deletions contrib/windows/glpi-agent-packaging.pl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use File::Spec;
use Cwd qw(abs_path);
use File::Spec::Functions qw(catfile);
use Data::UUID;

use constant {
PACKAGE_REVISION => "1", #BEWARE: always start with 1
Expand Down Expand Up @@ -90,6 +91,7 @@ sub build_app {
agent_vertag => $versiontag // '',
agent_fullname => $provider.' Agent',
agent_rootdir => $provider.'-Agent',
agent_localguid => Data::UUID->new()->create_str(),
agent_regpath => "Software\\$provider-Agent",
service_name => lc($provider).'-agent',
msi_sharedir => 'contrib/windows/packaging',
Expand Down Expand Up @@ -399,14 +401,14 @@ sub _tree2xml {
$feat = $self->_get_dir_feature($dir_id);
$result .= $ident ." ". qq[<Component Id="$component_id" Guid="{$component_guid}" KeyPath="yes" Feature="$feat">\n];
if ($dir_id eq 'd_install') {
$result .= $ident ." ". qq[ <CreateFolder>\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="CREATOR OWNER" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="LocalSystem" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="Administrators" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericWrite="no" GenericExecute="yes" GenericRead="yes" User="AuthenticatedUser" />\n];
$result .= $ident ." ". qq[ </CreateFolder>\n];
$result .= $ident ." ". qq[ <CreateFolder>\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="CREATOR OWNER" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="LocalSystem" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="Administrators" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericWrite="no" GenericExecute="yes" GenericRead="yes" User="AuthenticatedUser" />\n];
$result .= $ident ." ". qq[ </CreateFolder>\n];
} else {
$result .= $ident ." ". qq[ <CreateFolder />\n];
$result .= $ident ." ". qq[ <CreateFolder />\n];
}
if ($dir_id eq 'd_var') {
$result .= $ident ." ". qq[ <util:RemoveFolderEx On="uninstall" Property="UNINSTALL_VAR" />\n];
Expand All @@ -418,23 +420,6 @@ sub _tree2xml {
$result .= $ident ." ". qq[ <RemoveFolder Id="rm.$dir_id" On="uninstall" />\n];
}
$result .= $ident ." ". qq[</Component>\n];
# Also add virtual folder properties under d_install
if ($dir_id eq 'd_install') {
foreach my $id (qw(LOCAL)) {
$result .= $ident ." ". qq[<Directory Id="$id">\n];
($component_id, $component_guid) = $self->_gen_component_id(lc($id).".create");
$result .= $ident ." ". qq[<Component Id="$component_id" Guid="{$component_guid}" KeyPath="yes" Feature="$feat">\n];
$result .= $ident ." ". qq[ <CreateFolder>\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="CREATOR OWNER" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="LocalSystem" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericAll="yes" User="Administrators" />\n];
$result .= $ident ." ". qq[ <util:PermissionEx GenericWrite="no" GenericExecute="yes" GenericRead="yes" User="AuthenticatedUser" />\n];
$result .= $ident ." ". qq[ </CreateFolder>\n];
$result .= $ident ." ". qq[ <RemoveFolder Id="rm.] .lc($id). qq[" On="uninstall" />\n];
$result .= $ident ." ". qq[</Component>\n];
$result .= $ident ." ". qq[</Directory>\n];
}
}
}

if (scalar(@f) > 0) {
Expand Down
33 changes: 26 additions & 7 deletions contrib/windows/packaging/MSI_main-v2.wxs.tt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,9 @@
<RegistrySearch Id="Local" Root="HKLM" Key="[%agent_regpath%]" Name="local" Type="raw"/>
</Property>
<SetProperty Id="CMDLINE_LOCAL" Before="AppSearch" Value="[LOCAL]" />
<SetProperty Id="LOCAL" After="AppSearch" Value="[CMDLINE_LOCAL]"><![CDATA[CMDLINE_LOCAL<>"" OR CMDLINE_CONFIG="reset"]]></SetProperty>
<!-- Also compare to ProgramFiles64Folder to fix wrongly set LOCAL in 1.7.2 -->
<SetProperty Id="LOCAL" After="AppSearch" Value="[CMDLINE_LOCAL]"><![CDATA[CMDLINE_LOCAL<>"" OR LOCAL=INSTALLDIR OR CMDLINE_CONFIG="reset"]]></SetProperty>
<SetDirectory Id="_LOCALDIR" Before="CostFinalize" Value="[LOCAL]" />

<Property Id="ADDITIONAL_CONTENT" Secure="yes">
<RegistrySearch Id="AdditionalContent" Root="HKLM" Key="[%agent_regpath%]" Name="additional-content" Type="raw"/>
Expand Down Expand Up @@ -462,15 +464,19 @@
<LaunchConditions Sequence="200" />

[%- IF bits==32 %]
<Custom Action="SchedSecureObjects" After="CreateFolders"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
<Custom Action="SchedSecureObjects" After="CreateFolders"><![CDATA[NOT INSTALLDIR<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="SetFixInstallDir" After="SchedSecureObjects"><![CDATA[NOT INSTALLDIR<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="FixInstallDir" After="SetFixInstallDir"><![CDATA[NOT INSTALLDIR<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="SetFixLocalDir" After="SchedSecureObjects"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="FixLocalDir" After="SetFixLocalDir"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
[%- ELSE %]
<Custom Action="SchedSecureObjects_x64" After="CreateFolders"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
<Custom Action="SchedSecureObjects_x64" After="CreateFolders"><![CDATA[NOT INSTALLDIR<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="SetFixInstallDir" After="SchedSecureObjects_x64"><![CDATA[NOT INSTALLDIR<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="FixInstallDir" After="SetFixInstallDir"><![CDATA[NOT INSTALLDIR<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="SetFixLocalDir" After="SchedSecureObjects_x64"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFiles64Folder AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="FixLocalDir" After="SetFixLocalDir"><![CDATA[LOCAL<>"" AND NOT LOCAL<<ProgramFilesFolder AND NOT REMOVE~="ALL"]]></Custom>
[%- END %]
<Custom Action="SetFixInstallDir" After="SchedSecureObjects"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
<Custom Action="FixInstallDir" After="SetFixInstallDir"><![CDATA[NOT REMOVE~="ALL"]]></Custom>
<Custom Action="UpdateLocalDir" Before="CostFinalize"><![CDATA[LOCAL<>"" AND NOT LOCAL>>"\" AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="SetFixLocalDir" After="SchedSecureObjects"><![CDATA[LOCAL<>"" AND NOT REMOVE~="ALL"]]></Custom>
<Custom Action="FixLocalDir" After="SetFixLocalDir"><![CDATA[LOCAL<>"" AND NOT REMOVE~="ALL"]]></Custom>

<!-- Schedule custom action to always remove windows task if exists -->
<Custom Action="SetEndTask" Before="EndTask" />
Expand Down Expand Up @@ -1030,6 +1036,19 @@
</Directory> <!-- INSTALLDIR -->
</Directory> <!-- ProgramFilesFolder -->

<Directory Id="_LOCALDIR">
<Component Id="LocalDir" Guid="$(var.LocalDirGuid)" KeyPath="yes" Feature="feat_AGENT">
<CreateFolder>
<util:PermissionEx GenericAll="yes" User="CREATOR OWNER" />
<util:PermissionEx GenericAll="yes" User="LocalSystem" />
<util:PermissionEx GenericAll="yes" User="Administrators" />
<util:PermissionEx GenericExecute="yes" GenericRead="yes" User="AuthenticatedUser" />
</CreateFolder>
<RemoveFolder Id="rm.local" On="uninstall" />
<Condition><![CDATA[LOCAL<>""]]></Condition>
</Component>
</Directory>

<Directory Id="ProgramMenuFolder" />

</Directory> <!-- TARGETDIR -->
Expand Down
1 change: 1 addition & 0 deletions contrib/windows/packaging/Variables-v2.wxi.tt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
<?define URLAbout = "[%url_about%]" ?> <!-- e.g. "https://glpi-project.org/" -->
<?define URLHelp = "[%url_help%]" ?> <!-- e.g. "https://glpi-project.org/discussions/" -->
<?define RootDir = "[%agent_rootdir%]" ?> <!-- e.g. "GLPI-Agent" -->
<?define LocalDirGuid = "[%agent_localguid%]" ?>

<?define FileMainIcon = "[%msi_main_icon%]" ?>
<?define FileLicenseRtf = "[%msi_license_rtf%]" ?>
Expand Down

0 comments on commit 7d37cf6

Please sign in to comment.