From patchwork Thu Jan 9 19:04:36 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Adolf Belka X-Patchwork-Id: 8390 Return-Path: Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) client-signature RSA-PSS (4096 bits)) (Client CN "mail01.haj.ipfire.org", Issuer "R11" (verified OK)) by web04.haj.ipfire.org (Postfix) with ESMTPS id 4YTZ3f2Hwgz3x61 for ; Thu, 9 Jan 2025 19:05:02 +0000 (UTC) Received: from mail02.haj.ipfire.org (mail02.haj.ipfire.org [172.28.1.201]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature ECDSA (secp384r1)) (Client CN "mail02.haj.ipfire.org", Issuer "E5" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 4YTZ3Z0ps5z4g3; Thu, 9 Jan 2025 19:04:58 +0000 (UTC) Received: from mail02.haj.ipfire.org (localhost [127.0.0.1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4YTZ3Y5frhz340V; Thu, 9 Jan 2025 19:04:57 +0000 (UTC) Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) client-signature RSA-PSS (4096 bits)) (Client CN "mail01.haj.ipfire.org", Issuer "R11" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4YTZ3W2Ld1z33GG for ; Thu, 9 Jan 2025 19:04:55 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 4YTZ3V5Ps3z34q; Thu, 9 Jan 2025 19:04:54 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1736449494; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XE+lRitV3c98wMXToXmuCaqPtza6jJthAR+vmdcxbhU=; b=1ZOZJpJ069XHbhxLTQkM1egof2xVuz0Xy8o+ANitFn7SuEZEaaFpkHyTUAbC9sokvMVSux LSddkPmaTOxgzUBQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1736449494; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=XE+lRitV3c98wMXToXmuCaqPtza6jJthAR+vmdcxbhU=; b=DwbdOqv2qbrVJnqnCDHg7Gm5s2R77xK8tPnrp9sdv9XY+UORF8WwyGmcYtGX+dDJqg4gMf SyIaBOIiHA+L4QtS55Rb2stb8Nk7JcUjdeko8yRX1WO9I8clmuqu4eRVh92aJRDMCjwQSS /6FeGCFG3l0ToSSE2rPYYOabed2OVWki0mFLXxp6QCbNPZcKm1C9HlB3o7Dcg67qperjy5 pygYBnADeOFPRyvQH6UwGdQJyXLox7b13GAjhebc150plVlIHvEfpyBZDuB3GsSvkj40bb zdVBN1J6V7P4Pm7pehDg9ZBUYmnqV1GObKqy7sq67cyb8V16Z208pBkReoZYuA== From: Adolf Belka 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, 9 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> References: <20250109190441.18122-1-adolf.belka@ipfire.org> MIME-Version: 1.0 Message-ID-Hash: 46JWBJ65YCK4F3VPFEYE6EMLCMB5DJ5N X-Message-ID-Hash: 46JWBJ65YCK4F3VPFEYE6EMLCMB5DJ5N X-MailFrom: adolf.belka@ipfire.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.8 Precedence: list List-Id: IPFire development talk Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: - 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 Signed-off-by: Adolf Belka --- 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);