- This v2 version now includes the use of File-LibMagic to identify the specific content type and apply that to the modified header command so that image/png or image/jp[eg are used depending on the type of image provided. - Something changed in some package in CU188 that means that the existing method of printing the content type to the browser no longer worked. - I tested it in some stand alone code and even if using text/txt for the content-type print statement the File::Copy::copy then resulted in an Internal Server Error with the same message as with the image file which was "malformed header from script 'logo.cgi': Bad header:". - I tested it with text, html, image and application. In all cases the error message about a bad header was provided. - Did some searching and found an alternative way to explicitly print the header info which is what I have used in this patch change. - With this approach, in the stand alone code, I was able to get an image, html code or text shown in the browser correctly and without any error message. - I then used this new method in the logo.cgi code as submitted here and tested the change in my vm testbed and the image was shown in the captive portal correctly. - So this change fixes the problem with the logo not being shown but I have been unable to identify what changed to stop the method that worked prior to CU188 from working any more.
Fixes: Bug13795 Tested-by: Adolf Belka adolf.belka@ipfire.org Signed-off-by: Adolf Belka adolf.belka@ipfire.org --- html/cgi-bin/captive/logo.cgi | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/html/cgi-bin/captive/logo.cgi b/html/cgi-bin/captive/logo.cgi index 8f292b171..0b09476aa 100644 --- a/html/cgi-bin/captive/logo.cgi +++ b/html/cgi-bin/captive/logo.cgi @@ -2,9 +2,9 @@ ############################################################################### # # # IPFire.org - A linux based firewall # -# Copyright (C) 2016 Alexander Marx alexander.marx@ipfire.org # +# Copyright (C) 2016-2024 IPFire Team info@ipfire.org # # # -# This program is free software you can redistribute it and/or modify # +# This program is free software: you can redistribute it and/or modify # # it under the terms of the GNU General Public License as published by # # the Free Software Foundation, either version 3 of the License, or # # (at your option) any later version. # @@ -22,6 +22,7 @@ use strict; use CGI; use File::Copy; +use File::LibMagic;
# enable only the following on debugging purpose #use warnings; @@ -29,7 +30,11 @@ use File::Copy;
require '/var/ipfire/general-functions.pl';
+my $q = new CGI; +my $magic = File::LibMagic->new; + my $logo = "${General::swroot}/captive/logo.dat"; +my $file_info = $magic->info_from_filename($logo);
# Send 404 if logo was not uploaded and exit if (!-e $logo) { @@ -37,8 +42,8 @@ if (!-e $logo) { exit(0); }
-print "Content-Type: application/octet-stream\n\n"; - # Send image data +print $q->header(-type=>$file_info->{mime_type}); +binmode STDOUT; File::Copy::copy $logo, *STDOUT; exit(0);
- 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);
- It was placed in make.sh after perl-Config-AutoConf as that package is at least one build dependency.
Fixes: Bug13795 Tested-by: Adolf Belka adolf.belka@ipfire.org Signed-off-by: Adolf Belka adolf.belka@ipfire.org --- config/rootfiles/common/perl-File-LibMagic | 10 +++ lfs/perl-File-LibMagic | 77 ++++++++++++++++++++++ make.sh | 1 + 3 files changed, 88 insertions(+) create mode 100644 config/rootfiles/common/perl-File-LibMagic create mode 100644 lfs/perl-File-LibMagic
diff --git a/config/rootfiles/common/perl-File-LibMagic b/config/rootfiles/common/perl-File-LibMagic new file mode 100644 index 000000000..f08929963 --- /dev/null +++ b/config/rootfiles/common/perl-File-LibMagic @@ -0,0 +1,10 @@ +#usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/File +#usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/File/LibMagic +usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/File/LibMagic.pm +usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/File/LibMagic/Constants.pm +#usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/auto/File +#usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/auto/File/LibMagic +#usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/auto/File/LibMagic/.packlist +usr/lib/perl5/site_perl/5.36.0/xxxMACHINExxx-linux-thread-multi/auto/File/LibMagic/LibMagic.so +#usr/share/man/man3/File::LibMagic.3 +#usr/share/man/man3/File::LibMagic::Constants.3 diff --git a/lfs/perl-File-LibMagic b/lfs/perl-File-LibMagic new file mode 100644 index 000000000..f10a509fe --- /dev/null +++ b/lfs/perl-File-LibMagic @@ -0,0 +1,77 @@ +############################################################################### +# # +# IPFire.org - A linux based firewall # +# Copyright (C) 2025 IPFire Team info@ipfire.org # +# # +# This program is free software: you can redistribute it and/or modify # +# it under the terms of the GNU General Public License as published by # +# the Free Software Foundation, either version 3 of the License, or # +# (at your option) any later version. # +# # +# This program is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty of # +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # +# GNU General Public License for more details. # +# # +# You should have received a copy of the GNU General Public License # +# along with this program. If not, see http://www.gnu.org/licenses/. # +# # +############################################################################### + +############################################################################### +# Definitions +############################################################################### + +include Config + +VER = 1.23 + +THISAPP = File-LibMagic-$(VER) +DL_FILE = $(THISAPP).tar.gz +DL_FROM = $(URL_IPFIRE) +DIR_APP = $(DIR_SRC)/$(THISAPP) +TARGET = $(DIR_INFO)/$(THISAPP) + +############################################################################### +# Top-level Rules +############################################################################### + +objects = $(DL_FILE) + +$(DL_FILE) = $(DL_FROM)/$(DL_FILE) + +$(DL_FILE)_BLAKE2 = a409cdfbb7ac448858202ad79ee7b5cceb7d0bd17e42de108818ca6b03e8f8688f15dd5b5b0adc8ccab1a97174b02ccd93d5660dce2c04f585449182bd25a2aa + +install : $(TARGET) + +check : $(patsubst %,$(DIR_CHK)/%,$(objects)) + +download :$(patsubst %,$(DIR_DL)/%,$(objects)) + +b2 : $(subst %,%_BLAKE2,$(objects)) + +############################################################################### +# Downloading, checking, b2sum +############################################################################### + +$(patsubst %,$(DIR_CHK)/%,$(objects)) : + @$(CHECK) + +$(patsubst %,$(DIR_DL)/%,$(objects)) : + @$(LOAD) + +$(subst %,%_BLAKE2,$(objects)) : + @$(B2SUM) + +############################################################################### +# Installation Details +############################################################################### + +$(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects)) + @$(PREBUILD) + @rm -rf $(DIR_APP) && cd $(DIR_SRC) && tar zxf $(DIR_DL)/$(DL_FILE) + cd $(DIR_APP) && perl Makefile.PL + cd $(DIR_APP) && make $(MAKETUNING) + cd $(DIR_APP) && make install + @rm -rf $(DIR_APP) + @$(POSTBUILD) diff --git a/make.sh b/make.sh index 41bb1ea93..12a26d6df 100755 --- a/make.sh +++ b/make.sh @@ -1748,6 +1748,7 @@ build_system() { lfsmake2 perl-Net-Telnet lfsmake2 perl-Capture-Tiny lfsmake2 perl-Config-AutoConf + lfsmake2 perl-File-LibMagic lfsmake2 perl-Object-Tiny lfsmake2 perl-Archive-Peek-Libarchive lfsmake2 python3-inotify
- Changed the phrase in the code from Captive wrong ext to Captive wrong type as it is now the type and not the extension that is being checked.
Fixes: Bug13795 Tested-by: Adolf Belka adolf.belka@ipfire.org Signed-off-by: Adolf Belka adolf.belka@ipfire.org --- doc/language_issues.en | 2 +- doc/language_issues.it | 2 +- doc/language_issues.nl | 2 +- doc/language_issues.pl | 2 +- doc/language_issues.ru | 2 +- doc/language_missings | 8 ++++---- langs/de/cgi-bin/de.pl | 2 +- langs/en/cgi-bin/en.pl | 2 +- langs/es/cgi-bin/es.pl | 2 +- langs/fr/cgi-bin/fr.pl | 2 +- langs/tr/cgi-bin/tr.pl | 2 +- 11 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/doc/language_issues.en b/doc/language_issues.en index 3f1626b68..a1730ac7b 100644 --- a/doc/language_issues.en +++ b/doc/language_issues.en @@ -37,7 +37,7 @@ WARNING: untranslated string: Captive upload logo = Upload Logo WARNING: untranslated string: Captive upload logo recommendations = (PNG or JPEG, recommended 1280x720 pixels) WARNING: untranslated string: Captive valid for = Valid for WARNING: untranslated string: Captive vouchervalid = Allowed time for this coupon -WARNING: untranslated string: Captive wrong ext = Uploaded file has wrong filetype +WARNING: untranslated string: Captive wrong type = Uploaded file has wrong filetype WARNING: untranslated string: Choose Rule = Choose <u>one</u> of the following rules. WARNING: untranslated string: Class = Class WARNING: untranslated string: Class was deleted = with potential subclasses was deleted diff --git a/doc/language_issues.it b/doc/language_issues.it index 5870e2bc7..16371b566 100644 --- a/doc/language_issues.it +++ b/doc/language_issues.it @@ -936,7 +936,7 @@ WARNING: untranslated string: Captive upload logo = Upload Logo WARNING: untranslated string: Captive upload logo recommendations = (PNG or JPEG, recommended 1280x720 pixels) WARNING: untranslated string: Captive valid for = Valid for WARNING: untranslated string: Captive vouchervalid = Allowed time for this coupon -WARNING: untranslated string: Captive wrong ext = Uploaded file has wrong filetype +WARNING: untranslated string: Captive wrong type = Uploaded file has wrong filetype WARNING: untranslated string: MTU settings = MTU settings: WARNING: untranslated string: Number of Countries for the pie chart = Number of Countries for the pie chart WARNING: untranslated string: access point name = Access Point Name diff --git a/doc/language_issues.nl b/doc/language_issues.nl index 88493d1d9..f647d50a8 100644 --- a/doc/language_issues.nl +++ b/doc/language_issues.nl @@ -937,7 +937,7 @@ WARNING: untranslated string: Captive upload logo = Upload Logo WARNING: untranslated string: Captive upload logo recommendations = (PNG or JPEG, recommended 1280x720 pixels) WARNING: untranslated string: Captive valid for = Valid for WARNING: untranslated string: Captive vouchervalid = Allowed time for this coupon -WARNING: untranslated string: Captive wrong ext = Uploaded file has wrong filetype +WARNING: untranslated string: Captive wrong type = Uploaded file has wrong filetype WARNING: untranslated string: MTU settings = MTU settings: WARNING: untranslated string: Number of Countries for the pie chart = Number of Countries for the pie chart WARNING: untranslated string: access point name = Access Point Name diff --git a/doc/language_issues.pl b/doc/language_issues.pl index 5f3806102..a3acc61af 100644 --- a/doc/language_issues.pl +++ b/doc/language_issues.pl @@ -847,7 +847,7 @@ WARNING: untranslated string: Captive upload logo = Upload Logo WARNING: untranslated string: Captive upload logo recommendations = (PNG or JPEG, recommended 1280x720 pixels) WARNING: untranslated string: Captive valid for = Valid for WARNING: untranslated string: Captive vouchervalid = Allowed time for this coupon -WARNING: untranslated string: Captive wrong ext = Uploaded file has wrong filetype +WARNING: untranslated string: Captive wrong type = Uploaded file has wrong filetype WARNING: untranslated string: ConnSched dial = Connect WARNING: untranslated string: ConnSched hangup = Disconnect WARNING: untranslated string: ConnSched reboot = Reboot diff --git a/doc/language_issues.ru b/doc/language_issues.ru index 8891ce20e..e946c22df 100644 --- a/doc/language_issues.ru +++ b/doc/language_issues.ru @@ -841,7 +841,7 @@ WARNING: untranslated string: Captive upload logo = Upload Logo WARNING: untranslated string: Captive upload logo recommendations = (PNG or JPEG, recommended 1280x720 pixels) WARNING: untranslated string: Captive valid for = Valid for WARNING: untranslated string: Captive vouchervalid = Allowed time for this coupon -WARNING: untranslated string: Captive wrong ext = Uploaded file has wrong filetype +WARNING: untranslated string: Captive wrong type = Uploaded file has wrong filetype WARNING: untranslated string: ConnSched dial = Connect WARNING: untranslated string: ConnSched hangup = Disconnect WARNING: untranslated string: ConnSched reboot = Reboot diff --git a/doc/language_missings b/doc/language_missings index 2a2333d94..92a78b090 100644 --- a/doc/language_missings +++ b/doc/language_missings @@ -314,7 +314,7 @@ < Captive vouchervalid < Captive vout < Captive WiFi coupon -< Captive wrong ext +< Captive wrong type < check all < core update < cpu frequency @@ -858,7 +858,7 @@ < Captive vouchervalid < Captive vout < Captive WiFi coupon -< Captive wrong ext +< Captive wrong type < check all < cpu frequency < crypto error @@ -1441,7 +1441,7 @@ < Captive vouchervalid < Captive vout < Captive WiFi coupon -< Captive wrong ext +< Captive wrong type < ccd add < ccd choose net < ccd clientip @@ -2457,7 +2457,7 @@ < Captive vouchervalid < Captive vout < Captive WiFi coupon -< Captive wrong ext +< Captive wrong type < ccd add < ccd choose net < ccd clientip diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl index 2565505a5..5f89c7010 100644 --- a/langs/de/cgi-bin/de.pl +++ b/langs/de/cgi-bin/de.pl @@ -62,7 +62,7 @@ 'Captive voucher' => 'Gutschein', 'Captive vouchervalid' => 'Erlaubter Zeitraum für Gutschein', 'Captive vout' => 'Ausgegebene Gutscheine', -'Captive wrong ext' => 'Die hochgeladene Datei ist vom falschen Dateityp', +'Captive wrong type' => 'Die hochgeladene Datei ist vom falschen Dateityp', 'Choose Rule' => 'Wählen Sie <u>eine</u> der untenstehenden Regeln aus.', 'Class' => 'Klasse', 'Class was deleted' => 'wurde mit eventuell vorhandenen Unterklassen gelöscht', diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl index 5c8da52be..197f44633 100644 --- a/langs/en/cgi-bin/en.pl +++ b/langs/en/cgi-bin/en.pl @@ -61,7 +61,7 @@ 'Captive voucher' => 'Coupon', 'Captive vouchervalid' => 'Allowed time for this coupon', 'Captive vout' => 'Issued Vouchers', -'Captive wrong ext' => 'Uploaded file has wrong filetype', +'Captive wrong type' => 'Uploaded file has wrong filetype', 'Choose Rule' => 'Choose <u>one</u> of the following rules.', 'Class' => 'Class', 'Class was deleted' => 'with potential subclasses was deleted', diff --git a/langs/es/cgi-bin/es.pl b/langs/es/cgi-bin/es.pl index 27b0e739e..8a6322abe 100644 --- a/langs/es/cgi-bin/es.pl +++ b/langs/es/cgi-bin/es.pl @@ -61,7 +61,7 @@ 'Captive voucher' => 'Cupón', 'Captive vouchervalid' => 'Tiempo permitido para este cupón', 'Captive vout' => 'Cupones emitidos', -'Captive wrong ext' => 'El archivo subido tiene un tipo de archivo incorrecto', +'Captive wrong type' => 'El archivo subido tiene un tipo de archivo incorrecto', 'Choose Rule' => 'Seleccione <u>una</u> de las siguientes reglas.', 'Class' => 'Clase', 'Class was deleted' => 'con subclases potenciales fué eliminado', diff --git a/langs/fr/cgi-bin/fr.pl b/langs/fr/cgi-bin/fr.pl index 1b6d89eed..586f7f658 100644 --- a/langs/fr/cgi-bin/fr.pl +++ b/langs/fr/cgi-bin/fr.pl @@ -66,7 +66,7 @@ 'Captive voucher' => 'Coupon', 'Captive vouchervalid' => 'Délai d'expiration de ce coupon', 'Captive vout' => 'Reçus émis', -'Captive wrong ext' => 'Le fichier téléchargé a un mauvais type de fichier', +'Captive wrong type' => 'Le fichier téléchargé a un mauvais type de fichier', 'Choose Rule' => 'Choisissez <u>une</u> des règles suivantes.', 'Class' => 'Classe', 'Class was deleted' => 'Avec potentielles sous-classes effacées', diff --git a/langs/tr/cgi-bin/tr.pl b/langs/tr/cgi-bin/tr.pl index f3a37182a..baa1d39a5 100644 --- a/langs/tr/cgi-bin/tr.pl +++ b/langs/tr/cgi-bin/tr.pl @@ -61,7 +61,7 @@ 'Captive voucher' => 'Kupon', 'Captive vouchervalid' => 'Bu kupon için izin verilen süre', 'Captive vout' => 'Verilen Kuponlar', -'Captive wrong ext' => 'Yüklenen dosya yanlış dosya türüne sahip', +'Captive wrong type' => 'Yüklenen dosya yanlış dosya türüne sahip', 'Choose Rule' => 'Aşağıdaki kurallardan <u>birini</u> seçin.', 'Class' => 'Sınıf', 'Class was deleted' => 'Potansiyel alt sınıfları ile silindi',
-
Adolf Belka