captive.cgi: Update code to check for the image content type not just the extension

- The File-LibMagic used to do this content type check. As this requires the actual
   file and path name to access, the CGI::upload command had to be brought to before
   the content type check and download the file to /tmp/. Then the content type can be
   identified. If it is either image/png or image/jpeg then the logo.tmp file is
   moved to replace the existing logo.dat. If the uploaded logo is not a png or jpeg
   image content then the logo.tmp file in /tmp/ is deleted by unlinking it.
- I also added the actual content type to the error message if it is not a png or jpeg.
- Tested the code out on my vm testbed and it worked fine. Only png or jpeg content
   type is accepted It makes no difference what the extension on the file is. When not
   the correct content type the old logo.dat is left alone and not changed and the new
   logo stored in /tmp/ is removed. If the content type is correct then the new logo file
   in /tmp/ is moved to replace the existing logo.data file.
- When the wrong type of content was in the file, for example html code, then the error
   message is shown saying that the content type is not correct and showing the actual
   content type, in this case text/html.

Fixes: Bug13795
Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Arne Fitzenreiter <arne_f@ipfire.org>

diff --git a/html/cgi-bin/captive.cgi b/html/cgi-bin/captive.cgi
index ce666381c16f82a0bb21889fce2c72c93df0f7c5..6face0bda8e7b9456df9f95bcd527ba1da8c9a77 100644 (file)
--- a/html/cgi-bin/captive.cgi
+++ b/html/cgi-bin/captive.cgi
@@ -25,6 +25,8 @@ use HTML::Entities();
 use File::Basename;
 use PDF::API2;
 use constant mm => 25.4 / 72;
+use File::LibMagic;
+use File::Copy;
 
 # enable only the following on debugging purpose
 #use warnings;
@@ -53,6 +55,7 @@ my $coupons = "${General::swroot}/captive/coupons";
 my %couponhash = ();
 
 my $logo = "${General::swroot}/captive/logo.dat";
+my $logotmp = "/tmp/logo.tmp";
 
 my %settings=();
 my %mainsettings;
@@ -92,13 +95,25 @@ if ($cgiparams{'ACTION'} eq "export-coupons") {
 if ($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
        my $file = $cgiparams{'logo'};
        if ($file) {
-               # Check if the file extension is PNG/JPEG
                chomp $file;
-
-               my ($name, $path, $ext) = fileparse($file, qr/\.[^.]*$/);
-               if ($ext ne ".png" && $ext ne ".jpg" && $ext ne ".jpeg") {
-                       $errormessage = $Lang::tr{'Captive wrong ext'};
+               # Save logo to /tmp/ to carry out content type check
+               my ($filehandle) = CGI::upload("logo");
+               open(FILE, ">$logotmp");
+               binmode $filehandle;
+               while (<$filehandle>) {
+                       print FILE;
+               }
+               close(FILE);
+               # Check if the file content type is PNG or JPEG
+               my $magic = File::LibMagic->new;
+               my $file_info = $magic->info_from_filename($logotmp);
+               my $file_mime_type = $file_info->{mime_type};
+               if ($file_mime_type ne "image/png" && $file_mime_type ne "image/jpeg") {
+                       $errormessage = $Lang::tr{'Captive wrong type'}." - ".$file_mime_type;
+                       # Remove temporary logo file if there was an error.
+                       unlink $logotmp;
                }
+
        }
 
        $settings{'ENABLE_GREEN'}               = $cgiparams{'ENABLE_GREEN'};
@@ -111,17 +126,8 @@ if ($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
        if (!$errormessage){
                #Check if we need to upload a new logo
                if ($file) {
-                       # Save logo
-                       my ($filehandle) = CGI::upload("logo");
-
-                       # XXX check filesize
-
-                       open(FILE, ">$logo");
-                       binmode $filehandle;
-                       while (<$filehandle>) {
-                               print FILE;
-                       }
-                       close(FILE);
+                       # Move uploaded logo file from /tmp/
+                       File::Copy::move $logotmp, $logo;
                }
 
                &General::writehash("$settingsfile", \%settings);