# This patch fixes OSA-2017-09, also known as CVE-2017-16921: An attacker who
# is logged into OTRS as an agent can manipulate form parameters and execute
# arbitrary shell commands with the permissions of the OTRS or web server user.
# URL: https://www.otrs.com/security-advisory-2017-09-security-update-otrs-framework/
# Closes: #883774

diff -Naur otrs2-5.0.16.orig/Kernel/System/Crypt/PGP.pm otrs2-5.0.16/Kernel/System/Crypt/PGP.pm
--- otrs2-5.0.16.orig/Kernel/System/Crypt/PGP.pm	2017-01-17 03:39:35.000000000 +0100
+++ otrs2-5.0.16/Kernel/System/Crypt/PGP.pm	2017-12-13 13:06:04.300264158 +0100
@@ -11,6 +11,8 @@
 use strict;
 use warnings;
 
+use Kernel::System::VariableCheck qw(:all);
+
 our @ObjectDependencies = (
     'Kernel::Config',
     'Kernel::System::Encode',
@@ -110,6 +112,9 @@
         }
     }
 
+    # Quote the key parameter before passing it to the shell.
+	my $QuotedKey = $Self->_QuoteShellArgument( $Param{Key} );
+
     $Kernel::OM->Get('Kernel::System::Encode')->EncodeOutput( \$Param{Message} );
 
     # get temp file object
@@ -121,7 +126,7 @@
 
     my ( $FHCrypt, $FilenameCrypt ) = $FileTempObject->TempFile();
     close $FHCrypt;
-    my $GPGOptions = "--always-trust --yes --encrypt --armor -o $FilenameCrypt -r $Param{Key} $Filename";
+    my $GPGOptions = "--always-trust --yes --encrypt --armor -o $FilenameCrypt -r $QuotedKey $Filename";
     my $LogMessage = qx{$Self->{GPGBin} $GPGOptions 2>&1};
 
     # get crypted content
@@ -241,7 +246,11 @@
     my ( $FHPhrase, $FilePhrase ) = $FileTempObject->TempFile();
     print $FHPhrase $Pw;
     close $FHPhrase;
-    my $GPGOptions = qq{--passphrase-fd 0 --default-key $Param{Key} -o $FileSign $SigType $Filename};
+
+	# Quote the key parameter before passing it to the shell.
+	my $QuotedKey = $Self->_QuoteShellArgument( $Param{Key} );
+
+    my $GPGOptions = qq{--passphrase-fd 0 --default-key $QuotedKey -o $FileSign $SigType $Filename};
     my $LogMessage = qx{$Self->{GPGBin} $GPGOptions <$FilePhrase 2>&1};
 
     # error
@@ -638,7 +647,7 @@
 sub PrivateKeySearch {
     my ( $Self, %Param ) = @_;
 
-    my $Search         = $Param{Search} || '';
+	my $Search         = $Self->_QuoteShellArgument( $Param{Search} ) || '';
     my $GPGOptions     = "--list-secret-keys --with-fingerprint --with-colons $Search";
     my @GPGOutputLines = qx{$Self->{GPGBin} $GPGOptions 2>&1};
 
@@ -658,8 +667,8 @@
 sub PublicKeySearch {
     my ( $Self, %Param ) = @_;
 
-    my $Search         = $Param{Search} || '';
-    my $GPGOptions     = "--list-keys --with-fingerprint --with-colons $Search";
+	my $Search         = $Self->_QuoteShellArgument( $Param{Search} ) || '';
+	my $GPGOptions     = "--list-keys --with-fingerprint --with-colons $Search";
     my @GPGOutputLines = qx{$Self->{GPGBin} $GPGOptions 2>&1};
 
     return $Self->_ParseGPGKeyList( GPGOutputLines => \@GPGOutputLines );
@@ -678,8 +687,8 @@
 sub PublicKeyGet {
     my ( $Self, %Param ) = @_;
 
-    my $Key = quotemeta( $Param{Key} || '' );
-    my $LogMessage = qx{$Self->{GPGBin} --export --armor $Key 2>&1};
+	my $QuotedKey = $Self->_QuoteShellArgument( $Param{Key} ) || '';
+	my $LogMessage = qx{$Self->{GPGBin} --export --armor $QuotedKey 2>&1};
     my $PublicKey;
     if ( $LogMessage =~ /nothing exported/i ) {
         $LogMessage =~ s/\n//g;
@@ -719,9 +728,9 @@
 sub SecretKeyGet {
     my ( $Self, %Param ) = @_;
 
-    my $Key = quotemeta( $Param{Key} || '' );
+	my $QuotedKey  = $Self->_QuoteShellArgument( $Param{Key} ) || '';
 
-    my $LogMessage = qx{$Self->{GPGBin} --export-secret-keys --armor $Key 2>&1};
+    my $LogMessage = qx{$Self->{GPGBin} --export-secret-keys --armor $QuotedKey 2>&1};
     my $SecretKey  = '';
 
     if ( $LogMessage =~ /nothing exported/i ) {
@@ -769,9 +778,9 @@
         return;
     }
 
-    my $Key        = quotemeta( $Param{Key} || '' );
+	my $QuotedKey  = $Self->_QuoteShellArgument( $Param{Key} ) || '';
     my $GPGOptions = '--status-fd 1';
-    my $Message    = qx{$Self->{GPGBin} $GPGOptions --delete-key $Key 2>&1};
+	my $Message    = qx{$Self->{GPGBin} $GPGOptions --delete-key $QuotedKey 2>&1};
 
     my %LogMessage = $Self->_HandleLog( LogString => $Message );
 
@@ -932,7 +941,11 @@
     my ( $FHPhrase, $FilePhrase ) = $FileTempObject->TempFile();
     print $FHPhrase $Param{Password};
     close $FHPhrase;
-    my $GPGOptions = qq{--batch --passphrase-fd 0 --yes --decrypt -o $FileDecrypt $Param{Filename}};
+
+	# Quote the filename parameter before passing it to the shell.
+	my $QuotedFilename = $Self->_QuoteShellArgument( $Param{Filename} );
+
+    my $GPGOptions = qq{--batch --passphrase-fd 0 --yes --decrypt -o $FileDecrypt $QuotedFilename};
     my $LogMessage = qx{$Self->{GPGBin} $GPGOptions <$FilePhrase 2>&1};
     if ( $LogMessage =~ /failed/i ) {
         $Kernel::OM->Get('Kernel::System::Log')->Log(
@@ -1147,6 +1160,9 @@
         return;
     }
 
+	# Quote the file parameter before passing it to the shell.
+	my $QuotedFile = $Self->_QuoteShellArgument( $Param{File} );
+
     # This is a bit tricky: all we actually want is the list of keys that this message has been
     # encrypted for, but gpg does not seem to offer a way to just get these.
     # So we simply try to decrypt with an incorrect passphrase, which of course fails, but still
@@ -1155,7 +1171,7 @@
     my ( $FHPhrase, $FilePhrase ) = $Kernel::OM->Get('Kernel::System::FileTemp')->TempFile();
     print $FHPhrase '_no_this_is_not_the_@correct@_passphrase_';
     close $FHPhrase;
-    my $GPGOptions     = qq{--batch --passphrase-fd 0 --always-trust --yes --decrypt $Param{File}};
+    my $GPGOptions     = qq{--batch --passphrase-fd 0 --always-trust --yes --decrypt $QuotedFile};
     my @GPGOutputLines = qx{$Self->{GPGBin} $GPGOptions <$FilePhrase 2>&1};
 
     my @Keys;
@@ -1172,6 +1188,40 @@
     return @Keys;
 }
 
+
+=item _QuoteShellArgument()
+
+Quote passed string to be safe to use as a shell argument.
+
+    my $Result = $Self->_QuoteShellArgument(
+        "Safe string for 'shell arguments'."   # string to quote
+    );
+
+Returns quoted string if supplied or undef otherwise:
+
+    $Result = <<'EOS';
+'Safe string for '"'"'shell arguments'"'"'.'
+EOS
+
+=cut
+
+sub _QuoteShellArgument {
+    my ( $Self, $String ) = @_;
+
+    # Only continue with quoting if we received a valid string.
+    if ( IsStringWithData($String) ) {
+
+        # Encase any single quotes in double quotes, and glue them together with single quotes.
+        #   Please see https://stackoverflow.com/a/1250279 for more information.
+        $String =~ s/'/'"'"'/g;
+
+        # Enclose the string in single quotes.
+        return "'$String'";
+    }
+
+    return;
+}
+
 1;
 
 =end Internal: