Skip to content

Commit

Permalink
Removed configurable system commands from generic agents (CVE-2021-36100
Browse files Browse the repository at this point in the history
)
  • Loading branch information
jepf committed Mar 31, 2022
1 parent b3a9425 commit 309ec53
Show file tree
Hide file tree
Showing 12 changed files with 226 additions and 230 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
@@ -1,5 +1,6 @@
# 6.0.41 2022-xx-xx
- 2022-03-31 Fixed check of UntilTime in Kernel::System::Ticket::Event::TicketPendingTimeReset. [#221)(https://github.com/znuny/Znuny/issues/221)
- 2022-03-23 Removed configurable system commands from generic agents (CVE-2021-36100).
- 2022-03-14 Fixed sending notifications to invalid customer users.

# 6.0.40 2022-03-09
Expand Down
7 changes: 0 additions & 7 deletions Kernel/Config/Files/XML/Ticket.xml
Expand Up @@ -10396,13 +10396,6 @@
<Item ValueType="String" ValueRegex="^[0-9]{1,6}$">4000</Item>
</Value>
</Setting>
<Setting Name="Ticket::GenericAgentAllowCustomScriptExecution" Required="0" Valid="1">
<Description Translatable="1">Allows generic agent to execute custom command line scripts.</Description>
<Navigation>Core::Ticket</Navigation>
<Value>
<Item ValueType="Checkbox">1</Item>
</Value>
</Setting>
<Setting Name="Ticket::GenericAgentAllowCustomModuleExecution" Required="0" Valid="1">
<Description Translatable="1">Allows generic agent to execute custom modules.</Description>
<Navigation>Core::Ticket</Navigation>
Expand Down
3 changes: 1 addition & 2 deletions Kernel/Modules/AdminGenericAgent.pm
Expand Up @@ -112,7 +112,7 @@ sub Run {
NewParamValue1 NewParamValue2 NewParamValue3 NewParamValue4
NewParamKey5 NewParamKey6 NewParamKey7 NewParamKey8
NewParamValue5 NewParamValue6 NewParamValue7 NewParamValue8
NewLockID NewDelete NewCMD NewSendNoNotification NewArchiveFlag
NewLockID NewDelete NewSendNoNotification NewArchiveFlag
ScheduleLastRun Valid
)
)
Expand Down Expand Up @@ -899,7 +899,6 @@ sub _MaskUpdate {
Class => 'Modernize',
);

$JobData{AllowCustomScriptExecution} = $ConfigObject->Get('Ticket::GenericAgentAllowCustomScriptExecution') || 0;
$JobData{AllowCustomModuleExecution} = $ConfigObject->Get('Ticket::GenericAgentAllowCustomModuleExecution') || 0;

$LayoutObject->Block(
Expand Down
11 changes: 0 additions & 11 deletions Kernel/Output/HTML/Templates/Standard/AdminGenericAgent.tt
Expand Up @@ -861,17 +861,6 @@
</div>
<div class="Clear"></div>

[% IF Data.AllowCustomScriptExecution %]
<label for="NewCMD">[% Translate("CMD") | html %]:</label>
<div class="Field">
<input type="text" name="NewCMD" id="NewCMD" value="[% Data.NewCMD | html %]" size="40"/>
<p class="FieldExplanation">
([% Translate("This command will be executed. ARG[0] will be the ticket number. ARG[1] the ticket id.") | html %])
</p>
</div>
<div class="Clear"></div>
[% END %]

<label for="NewDelete">[% Translate("Delete tickets") | html %]:</label>
<div class="Field">
[% Data.DeleteOption %]
Expand Down
31 changes: 0 additions & 31 deletions Kernel/System/GenericAgent.pm
Expand Up @@ -1447,37 +1447,6 @@ sub _JobRunTicket {
);
}

# cmd
my $AllowCustomScriptExecution
= $Kernel::OM->Get('Kernel::Config')->Get('Ticket::GenericAgentAllowCustomScriptExecution') || 0;

if ( $Param{Config}->{New}->{CMD} && $AllowCustomScriptExecution ) {
if ( $Self->{NoticeSTDOUT} ) {
print " - Execute '$Param{Config}->{New}->{CMD}' for Ticket $Ticket.\n";
}
$Kernel::OM->Get('Kernel::System::Log')->Log(
Priority => 'notice',
Message => "Execute '$Param{Config}->{New}->{CMD}' for Ticket $Ticket.",
);
system("$Param{Config}->{New}->{CMD} $Param{TicketNumber} $Param{TicketID} ");

if ( $? ne 0 ) {
$Kernel::OM->Get('Kernel::System::Log')->Log(
Priority => 'notice',
Message => "Command returned a nonzero return code: rc=$?, err=$!",
);
}
}
elsif ( $Param{Config}->{New}->{CMD} && !$AllowCustomScriptExecution ) {
if ( $Self->{NoticeSTDOUT} ) {
print " - Execute '$Param{Config}->{New}->{CMD}' is not allowed by the system configuration..\n";
}
$Kernel::OM->Get('Kernel::System::Log')->Log(
Priority => 'error',
Message => "Execute '$Param{Config}->{New}->{CMD}' is not allowed by the system configuration..",
);
}

# delete ticket
if ( $Param{Config}->{New}->{Delete} ) {
if ( $Self->{NoticeSTDOUT} ) {
Expand Down
44 changes: 44 additions & 0 deletions Kernel/System/GenericAgent/SystemCommandExecution.pm.dist
@@ -0,0 +1,44 @@
# --
# Copyright (C) 2021-2022 Znuny GmbH, https://znuny.org/
# --
# This software comes with ABSOLUTELY NO WARRANTY. For details, see
# the enclosed file COPYING for license information (AGPL). If you
# did not receive this file, see http://www.gnu.org/licenses/agpl.txt.
# --

package Kernel::System::GenericAgent::SystemCommandExecution;

use strict;
use warnings;

our @ObjectDependencies;

#
# Example module to show the execution of system commands in generic agent context.
#

sub new {
my ( $Type, %Param ) = @_;

my $Self = {};
bless( $Self, $Type );

# 0=off; 1=on;
$Self->{Debug} = $Param{Debug} || 0;

return $Self;
}

sub Run {
my ( $Self, %Param ) = @_;

# Execute system command
my $Output = `/path/to/some/script.sh`;

# Parameters given in generic agent config can be used, e.g.:
$Output = `/path/to/some/script.sh $Param{TicketID}`;

return 1;
}

1;
4 changes: 4 additions & 0 deletions scripts/DBUpdateTo6.pm
Expand Up @@ -218,6 +218,10 @@ sub _TasksGet {
Message => 'Check if database has been backed up',
Module => 'DatabaseBackupCheck',
},
{
Message => 'Remove Generic Agent system commands',
Module => 'RemoveGenericAgentSystemCommandCalls',
},
{
Message => 'Upgrade database structure',
Module => 'UpgradeDatabaseStructure',
Expand Down
107 changes: 107 additions & 0 deletions scripts/DBUpdateTo6/RemoveGenericAgentSystemCommandCalls.pm
@@ -0,0 +1,107 @@
# --
# Copyright (C) 2021-2022 Znuny GmbH, https://znuny.org/
# --
# This software comes with ABSOLUTELY NO WARRANTY. For details, see
# the enclosed file COPYING for license information (AGPL). If you
# did not receive this file, see http://www.gnu.org/licenses/agpl.txt.
# --

package scripts::DBUpdateTo6::RemoveGenericAgentSystemCommandCalls; ## no critic

use strict;
use warnings;

use IO::Interactive qw(is_interactive);

use parent qw(scripts::DBUpdateTo6::Base);

use version;

use Kernel::System::VariableCheck qw(:all);

our @ObjectDependencies = (
'Kernel::System::GenericAgent',
);

sub Run {
my ( $Self, %Param ) = @_;

my $GenericAgentObject = $Kernel::OM->Get('Kernel::System::GenericAgent');

my $UserID = 1;

my $JobsToMigrate = $Self->_GetJobsToMigrate();
return 1 if !IsHashRefWithData($JobsToMigrate);

# Remove system commands from generic agents and mark them via job name as '[NEEDS ATTENTION]'
for my $OldJobName ( sort keys %{$JobsToMigrate} ) {
my $NewJobName = "[NEEDS ATTENTION] $OldJobName";

my $Job = $JobsToMigrate->{$OldJobName};
delete $Job->{NewCMD};

$Job->{Valid} = 0;

$GenericAgentObject->JobDelete(
Name => $OldJobName,
UserID => $UserID,
);

$GenericAgentObject->JobAdd(
Name => $NewJobName,
Data => $Job,
UserID => $UserID,
);
}

return 1;
}

sub CheckPreviousRequirement {
my ( $Self, %Param ) = @_;

my $JobsToMigrate = $Self->_GetJobsToMigrate();
return 1 if !IsHashRefWithData($JobsToMigrate);

print "\n The following generic agent jobs have configured system command calls.\n";
print
" System command calls are not allowed anymore and will be removed. The job will also be renamed and set invalid.\n";

for my $JobName ( sort keys %{$JobsToMigrate} ) {
print " $JobName: $JobsToMigrate->{$JobName}->{NewCMD}\n";
}

if ( is_interactive() ) {
print ' Do you want to continue? [Y]es/[N]o: ';

my $Answer = <>;
$Answer =~ s{\s}{}g;

return if $Answer !~ m{\Ay(es)?\z}i;
}

return 1;
}

sub _GetJobsToMigrate {
my ( $Self, %Param ) = @_;

my $GenericAgentObject = $Kernel::OM->Get('Kernel::System::GenericAgent');

my %Jobs = $GenericAgentObject->JobList();

my %JobsToMigrate;

JOBNAME:
for my $JobName ( sort keys %Jobs ) {
my %Job = $GenericAgentObject->JobGet( Name => $JobName );
next JOBNAME if !%Job;
next JOBNAME if !IsStringWithData( $Job{'NewCMD'} );

$JobsToMigrate{$JobName} = {%Job};
}

return \%JobsToMigrate;
}

1;
1 change: 0 additions & 1 deletion scripts/test/GenericAgent.t
Expand Up @@ -195,7 +195,6 @@ my %NewJob = (
NewLockID => 2,
DynamicField_TicketFreeKey2 => 'Test',
DynamicField_TicketFreeText2 => 'Value 2',
NewCMD => '',
NewParamKey1 => '',
NewParamValue1 => '',
NewParamKey2 => '',
Expand Down

0 comments on commit 309ec53

Please sign in to comment.