From mboxrd@z Thu Jan  1 00:00:00 1970
From: Adolf Belka <adolf.belka@ipfire.org>
To: development@lists.ipfire.org
Subject: [PATCH v2 2/4] captive.cgi: Update code to check for the image
 content type not just the extension
Date: Thu, 09 Jan 2025 20:04:36 +0100
Message-ID: <20250109190441.18122-2-adolf.belka@ipfire.org>
In-Reply-To: <20250109190441.18122-1-adolf.belka@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============7074016389696728394=="
List-Id: <development.lists.ipfire.org>

--===============7074016389696728394==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

- 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 ty=
pe 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 pn=
g or jpeg.
- Tested the code out on my vm testbed and it worked fine. Only png or jpeg c=
ontent
   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 an=
d the new
   logo stored in /tmp/ is removed. If the content type is correct then the n=
ew 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 t=
he actual
   content type, in this case text/html.

Fixes: Bug13795
Tested-by: Adolf Belka <adolf.belka(a)ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka(a)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 =3D> 25.4 / 72;
+use File::LibMagic;
+use File::Copy;
=20
 # enable only the following on debugging purpose
 #use warnings;
@@ -53,6 +55,7 @@ my $coupons =3D "${General::swroot}/captive/coupons";
 my %couponhash =3D ();
=20
 my $logo =3D "${General::swroot}/captive/logo.dat";
+my $logotmp =3D "/tmp/logo.tmp";
=20
 my %settings=3D();
 my %mainsettings;
@@ -92,13 +95,25 @@ if ($cgiparams{'ACTION'} eq "export-coupons") {
 if ($cgiparams{'ACTION'} eq $Lang::tr{'save'}) {
 	my $file =3D $cgiparams{'logo'};
 	if ($file) {
-		# Check if the file extension is PNG/JPEG
 		chomp $file;
-
-		my ($name, $path, $ext) =3D fileparse($file, qr/\.[^.]*$/);
-		if ($ext ne ".png" && $ext ne ".jpg" && $ext ne ".jpeg") {
-			$errormessage =3D $Lang::tr{'Captive wrong ext'};
+		# Save logo to /tmp/ to carry out content type check
+		my ($filehandle) =3D 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 =3D File::LibMagic->new;
+		my $file_info =3D $magic->info_from_filename($logotmp);
+		my $file_mime_type =3D $file_info->{mime_type};
+		if ($file_mime_type ne "image/png" && $file_mime_type ne "image/jpeg") {
+			$errormessage =3D $Lang::tr{'Captive wrong type'}." - ".$file_mime_type;
+			# Remove temporary logo file if there was an error.
+			unlink $logotmp;
 		}
+
 	}
=20
 	$settings{'ENABLE_GREEN'}		=3D $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) =3D 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;
 		}
=20
 		&General::writehash("$settingsfile", \%settings);
--=20
2.47.1


--===============7074016389696728394==--