- 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 --- html/cgi-bin/captive.cgi | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/html/cgi-bin/captive.cgi b/html/cgi-bin/captive.cgi index ce666381c..6face0bda 100644 --- 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);