[Bug 27286] New: Patron picture-upload.pl allows arbitrary file extension during upload
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Bug ID: 27286 Summary: Patron picture-upload.pl allows arbitrary file extension during upload Change sponsored?: --- Product: Koha Version: 20.05 Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Patrons Assignee: koha-bugs@lists.koha-community.org Reporter: saiful@semanticconsulting.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Created attachment 114546 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114546&action=edit Double extension or wrong extension in picture file upload The IT team has flagged two issues with the picture upload tool (Patron Images feature): 1. The tool allows upload of files with double extension. 2. The tool allows upload of files with arbitrary file extension. In essence, the tool does not validate the image file restriction that is mentioned in the top of upload input tag which is: "Only PNG, GIF, JPEG, XPM formats are supported." -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Saiful Amin <saiful@semanticconsulting.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch CC| |saiful@semanticconsulting.c | |om -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Saiful Amin <saiful@semanticconsulting.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |In Discussion --- Comment #1 from Saiful Amin <saiful@semanticconsulting.com> --- I have tested the following changes to fix both the issues. Need someone to test and approve (I don't have the full environment to submit the patch). /usr/share/koha/intranet/cgi-bin/tools/picture-upload.pl Line: 96++ output_and_exit( $input, $cookie, $template, 'wrong_image_file_ext' ) unless $uploadfilename =~ m/^[^\.]+\.(png|gif|jpg|jpeg|xpm)$/i; /usr/share/koha/intranet/htdocs/intranet-tmpl/prog/en/includes/blocking_errors.inc Line: 9++ [% CASE 'wrong_image_file_ext' %] <div class="dialog message">Wrong image file extension or double extension in file name</div> -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|In Discussion |ASSIGNED --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Resetting to NEW to make this a bit more visible! -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEW -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|20.05 |master -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- When I use the tool, there is a note: NOTE: Only PNG, GIF, JPEG, XPM formats are supported. Maximum image size is 2MB. And if I try to upload a .txt it fails giving a nice error. When I try the same on the patron record, the modal closes without any visible error. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Saiful Amin <saiful@semanticconsulting.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |saiful@semanticconsulting.c |ity.org |om -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 --- Comment #4 from Saiful Amin <saiful@semanticconsulting.com> --- Created attachment 189978 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=189978&action=edit Bug 27286: Patron picture-upload.pl allows arbitrary file extension during upload This patch introduces strict validation for filenames in the patron image upload tool. It addresses VAPT security concerns regarding "double extension" attacks (e.g., image.php.jpg or archive.php.zip). The patch ensures that: 1. Filenames containing more than one period are rejected. 2. Only strictly allowed extensions (zip, png, gif, jpg, jpeg, xpm) are accepted. Test Plan: 1. Pre-patch check: a. Go to Tools -> Upload patron images. b. Rename an image file to 'test.php.jpg'. c. Upload this file. d. Observe that the upload is accepted, but the filename itself is not flagged as invalid. e. Create a valid file named 'test.php.zip' (with valid content). f. Upload this file. g. Result: The upload proceeds. 2. Apply the patch. 3. Verify rejection (images): a. Go to Tools -> Upload patron images. b. Attempt to upload 'test.php.jpg'. c. Result: The tool stops immediately with a "BADFILENAME" error message. d. Attempt to upload 'test.v1.jpg' (valid image, extra dot). e. Result: The tool stops immediately with a "BADFILENAME" error message. 4. Verify rejection (zip): a. Attempt to upload 'archive.php.zip'. b. Result: The tool stops immediately with a "BADFILENAME" error message. 5. Verify valid upload: a. Attempt to upload 'patron.jpg' (standard valid file). b. Result: Upload succeeds. c. Attempt to upload 'patrons.zip' (standard valid zip). d. Result: Upload succeeds and files are unpacked. 6. Sign-off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Saiful Amin <saiful@semanticconsulting.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #189978|0 |1 is obsolete| | --- Comment #5 from David Nind <david@davidnind.com> --- Created attachment 190027 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=190027&action=edit Bug 27286: Patron picture-upload.pl allows arbitrary file extension during upload This patch introduces strict validation for filenames in the patron image upload tool. It addresses VAPT security concerns regarding "double extension" attacks (e.g., image.php.jpg or archive.php.zip). The patch ensures that: 1. Filenames containing more than one period are rejected. 2. Only strictly allowed extensions (zip, png, gif, jpg, jpeg, xpm) are accepted. Test Plan: 1. Pre-patch check: a. Go to Tools -> Upload patron images. b. Rename an image file to 'test.php.jpg'. c. Upload this file. d. Observe that the upload is accepted, but the filename itself is not flagged as invalid. e. Create a valid file named 'test.php.zip' (with valid content). f. Upload this file. g. Result: The upload proceeds. 2. Apply the patch. 3. Verify rejection (images): a. Go to Tools -> Upload patron images. b. Attempt to upload 'test.php.jpg'. c. Result: The tool stops immediately with a "BADFILENAME" error message. d. Attempt to upload 'test.v1.jpg' (valid image, extra dot). e. Result: The tool stops immediately with a "BADFILENAME" error message. 4. Verify rejection (zip): a. Attempt to upload 'archive.php.zip'. b. Result: The tool stops immediately with a "BADFILENAME" error message. 5. Verify valid upload: a. Attempt to upload 'patron.jpg' (standard valid file). b. Result: Upload succeeds. c. Attempt to upload 'patrons.zip' (standard valid zip). d. Result: Upload succeeds and files are unpacked. 6. Sign-off. Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #6 from David Nind <david@davidnind.com> --- Testing notes (using koha-testing-dcoker): 1. Enable the patronimages system preference (set to 'Allow'). I'm not on the QA team, but: 1. The QA errors need fixing (from running qa in koha-testing-docker): qa Tag v25.05.00-1 found in /kohadevbox/koha — checking out branch main in /kohadevbox/qa-test-tools Already on 'main' Your branch is up to date with 'origin/main'. testing 1 commit(s) (applied to 7adeda9 'f1 Bug 38426: yarn.lock changes') Processing files [RUNNING] koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt [RUNNING] tools/picture-upload.pl [PASS] tools/picture-upload.pl [FAIL] koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt Result: [FAIL] koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt` [PASS] tools/picture-upload.pl Processing additional checks PASS! * Was this bug sponsored by anyone? 2. I don't think it is necessary to have the comment "# Fix bug 27286" in the patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 --- Comment #7 from Saiful Amin <saiful@semanticconsulting.com> ---
Result: [FAIL] koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt` [PASS] tools/picture-upload.pl
2. I don't think it is necessary to have the comment "# Fix bug 27286" in the patch.
Do I need to resubmit the patch with the changes recommended or is the QA team allowed to do these changes themselves? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 --- Comment #8 from David Nind <david@davidnind.com> --- (In reply to Saiful Amin from comment #7)
Result: [FAIL] koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt FAIL tidiness File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/en/modules/tools/picture-upload.tt` [PASS] tools/picture-upload.pl
2. I don't think it is necessary to have the comment "# Fix bug 27286" in the patch.
Do I need to resubmit the patch with the changes recommended or is the QA team allowed to do these changes themselves?
The QA Team would normally ask you to make these changes. Add these in a follow-up patch - see https://wiki.koha-community.org/wiki/Commit_messages#Follow-ups For example: Bug 27286: (follow-up) Tidy code and remove comment -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Abdivasiyev Sunnatillo <abdivasiyevsunnatillo2008@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |abdivasiyevsunnatillo2008@g | |mail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27286 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #9 from Jonathan Druart <jonathan.druart@gmail.com> --- Shouldn't we look at the mimetype instead? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org