From d4e3dfbaa054762b29df54705aa412685dd37e15 Mon Sep 17 00:00:00 2001
From: Martin Gruner <martin.gruner@otrs.com>
Date: Sun, 16 Dec 2018 15:10:05 +0000
Subject: [PATCH] Fixed: Incorrect handling of content-type in PictureUpload
 (bug#14305).

---
 CHANGES.md                            |   1 +
 Kernel/Modules/PictureUpload.pm       |  22 +++-
 scripts/test/Frontend/PictureUpload.t | 147 ++++++++++++++++++++++++++
 3 files changed, 169 insertions(+), 1 deletion(-)
 create mode 100644 scripts/test/Frontend/PictureUpload.t

--- a/Kernel/Modules/PictureUpload.pm
+++ b/Kernel/Modules/PictureUpload.pm
@@ -65,6 +65,26 @@ sub Run {
         for my $Attachment (@AttachmentData) {
             next ATTACHMENT if !$Attachment->{ContentID};
             next ATTACHMENT if $Attachment->{ContentID} ne $ContentID;
+
+            if (
+                $Attachment->{Filename} !~ /\.(png|gif|jpg|jpeg|bmp)$/i
+                || substr( $Attachment->{ContentType}, 0, 6 ) ne 'image/'
+                )
+            {
+                $LayoutObject->Block(
+                    Name => 'ErrorNoImageFile',
+                    Data => {
+                        CKEditorFuncNum => $CKEditorFuncNum,
+                    },
+                );
+                return $LayoutObject->Attachment(
+                    ContentType => 'text/html; charset=' . $Charset,
+                    Content     => $LayoutObject->Output( TemplateFile => 'PictureUpload' ),
+                    Type        => 'inline',
+                    NoCache     => 1,
+                );
+            }
+
             return $LayoutObject->Attachment(
                 Type => 'inline',
                 %{$Attachment},
@@ -94,7 +114,7 @@ sub Run {
     }
 
     # return error if file is not possible to show inline
-    if ( $File{Filename} !~ /\.(png|gif|jpg|jpeg)$/i ) {
+    if ( $File{Filename} !~ /\.(png|gif|jpg|jpeg)$/i || substr( $File{ContentType}, 0, 6 ) ne 'image/' ) {
         $LayoutObject->Block(
             Name => 'ErrorNoImageFile',
             Data => {
--- /dev/null
+++ b/scripts/test/Frontend/PictureUpload.t
@@ -0,0 +1,147 @@
+# --
+# Copyright (C) 2001-2018 OTRS AG, https://otrs.com/
+# --
+# This software comes with ABSOLUTELY NO WARRANTY. For details, see
+# the enclosed file COPYING for license information (GPL). If you
+# did not receive this file, see https://www.gnu.org/licenses/gpl-3.0.txt.
+# --
+
+use strict;
+use warnings;
+use utf8;
+
+use vars (qw($Self));
+
+use LWP::UserAgent;
+
+use Kernel::System::UnitTest::Helper;
+
+my $ConfigObject = $Kernel::OM->Get('Kernel::Config');
+my $JSONObject   = $Kernel::OM->Get('Kernel::System::JSON');
+
+# get helper object
+$Kernel::OM->ObjectParamAdd(
+    'Kernel::System::UnitTest::Helper' => {
+        SkipSSLVerify => 1
+    },
+);
+my $Helper = $Kernel::OM->Get('Kernel::System::UnitTest::Helper');
+
+my $TestUserLogin         = $Helper->TestUserCreate();
+my $TestCustomerUserLogin = $Helper->TestCustomerUserCreate();
+
+my $BaseURL = $ConfigObject->Get('HttpType') . '://';
+
+$BaseURL .= $Helper->GetTestHTTPHostname() . '/';
+$BaseURL .= $ConfigObject->Get('ScriptAlias') . 'index.pl?';
+
+my $UserAgent = LWP::UserAgent->new(
+    Timeout => 60,
+);
+$UserAgent->cookie_jar( {} );    # keep cookies
+
+my $Response = $UserAgent->get(
+    $BaseURL . "Action=Login;User=$TestUserLogin;Password=$TestUserLogin;"
+);
+if ( !$Response->is_success() ) {
+    $Self->True(
+        0,
+        "Could not login to agent interface, aborting! URL: ${BaseURL}Action=Login;User=$TestUserLogin;Password=$TestUserLogin;"
+    );
+    return 1;
+}
+
+my $UploadCacheObject = $Kernel::OM->Get('Kernel::System::Web::UploadCache');
+my $FormID            = $UploadCacheObject->FormIDCreate();
+
+my $CheckUpload = sub {
+    my %Param = @_;
+
+    $Self->Is(
+        $Response->code(),
+        200,
+        "Response status is successful",
+    );
+
+    my $Method = $Param{Successful} ? 'True' : 'False';
+
+    $Self->$Method(
+        scalar $Response->content() =~ m{Action=PictureUpload},
+        "Response check for link to uploaded image",
+    );
+
+    if ( $Param{Successful} ) {
+        my ($ContentID) = $Response->content() =~ m{ContentID=(.*)"};
+
+        $Response = $UserAgent->get("${BaseURL}Action=PictureUpload;FormID=$FormID;ContentID=$ContentID");
+
+        $Self->Is(
+            $Response->content(),
+            $Param{Content},
+            'Response content',
+        );
+
+        $Self->Is(
+            substr( $Response->header('Content-Type'), 0, 6 ),
+            'image/',
+            'Response content type',
+        );
+    }
+};
+
+# Upload image correctly and verify it.
+$Response = $UserAgent->post(
+    $BaseURL,
+    Content_Type => 'form-data',
+    Content      => {
+        Action => 'PictureUpload',
+        FormID => $FormID,
+        upload => [
+            undef, 'index1.png',
+            'Content-Type' => 'image/png',
+            Content        => '123'
+        ],
+    }
+);
+
+$CheckUpload->(
+    Successful => 1,
+    Content    => '123'
+);
+
+# Upload image with wrong content-type, must fail.
+$Response = $UserAgent->post(
+    $BaseURL,
+    Content_Type => 'form-data',
+    Content      => {
+        Action => 'PictureUpload',
+        FormID => $FormID,
+        upload => [
+            undef, 'index2.png',
+            'Content-Type' => 'text/html',
+            Content        => '123'
+        ],
+    }
+);
+
+$CheckUpload->( Successful => 0 );
+
+# Store image with wrong content-type and verify that it is not served by the application.
+my $ContentID = 'inline695415.287823406.1544532925.8063442.1111111111@unittest.local';
+$UploadCacheObject->FormIDAddFile(
+    FormID      => $FormID,
+    Filename    => 'index3.png',
+    Content     => '123',
+    ContentID   => $ContentID,
+    ContentType => 'text/html',
+    Disposition => 'inline',       # optional
+);
+
+$Response = $UserAgent->get("${BaseURL}Action=PictureUpload;FormID=$FormID;ContentID=$ContentID");
+$Self->True(
+    index( $Response->content(),
+        q|CKEDITOR.tools.callFunction(0, '', "The file is not an image that can be shown inline!"| ) > -1,
+    'Response check for CKEditor error handler',
+);
+
+1;