As noted on bug 37323, in a datalink.txt file, you can craft a filepath to load any image on the system that is readable by that instance's Linux user.
Created attachment 169640 [details] [review] Bug 37488: Validate paths in datalink.txt/idlink.txt files This change validates the paths in datalink.txt/idlink.txt, so that only images in the unpacked archive directory are allowed Test plan: 0. Apply the patch 1. koha-plack --reload kohadev 2. Create a datalink.txt file with the following: 42,selfie.jpg 3. Create a jpeg at selfie.jpg 4. ZIP the datalink.txt and selfie.jpg files 5. Upload to the "Upload patron images" tool (after enabling the "patronimages" system preference) 6. Note that the image uploads correctly
Created attachment 169704 [details] [review] Bug 37488: Validate paths in datalink.txt/idlink.txt files This change validates the paths in datalink.txt/idlink.txt, so that only images in the unpacked archive directory are allowed Test plan: 0. Apply the patch 1. koha-plack --reload kohadev 2. Create a datalink.txt file with the following: 42,selfie.jpg 3. Create a jpeg at selfie.jpg 4. ZIP the datalink.txt and selfie.jpg files 5. Upload to the "Upload patron images" tool (after enabling the "patronimages" system preference) 6. Note that the image uploads correctly Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 169707 [details] Image file that can be used for testing Combination of helpfulness and vanity :-) Feel free to use my lovely selfie to ease testing
Looking here
Created attachment 169992 [details] [review] Bug 37488: Validate paths in datalink.txt/idlink.txt files This change validates the paths in datalink.txt/idlink.txt, so that only images in the unpacked archive directory are allowed Test plan: 0. Apply the patch 1. koha-plack --reload kohadev 2. Create a datalink.txt file with the following: 42,selfie.jpg 3. Create a jpeg at selfie.jpg 4. ZIP the datalink.txt and selfie.jpg files 5. Upload to the "Upload patron images" tool (after enabling the "patronimages" system preference) 6. Note that the image uploads correctly Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Isnt this patch only redirecting to another error message actually ? Without this patch, I have something like: [2024/08/02 08:58:38] [WARN] Opening /tmp/7PkHGkjawG//etc/selfie.jpg failed! at /usr/share/koha/tools/picture-upload.pl line 413. (Line number may be affected by a few debugs.) And with this patch: [2024/08/02 08:59:49] [WARN] Missing filename at /usr/share/koha/tools/picture-upload.pl line 435. Using a path /etc/selfie.jpg (nice picture, Nick ;) Will put a QA stamp on it, just asking.. Note that missing filename is not really the best error message?
(In reply to Marcel de Rooy from comment #7) > Without this patch, I have something like: > > [2024/08/02 08:58:38] [WARN] Opening /tmp/7PkHGkjawG//etc/selfie.jpg failed! > at /usr/share/koha/tools/picture-upload.pl line 413. > (Line number may be affected by a few debugs.) You haven't crafted your exploit quite right there. You'd need to preface "/etc/selfie.jpg" with "../.." and then it would work. > And with this patch: > > [2024/08/02 08:59:49] [WARN] Missing filename at > /usr/share/koha/tools/picture-upload.pl line 435. > > Using a path /etc/selfie.jpg (nice picture, Nick ;) > > Will put a QA stamp on it, just asking.. > > Note that missing filename is not really the best error message? No, it's probably not the best error message, but I thought it was accurate enough, and kept the patch small and easy to backport.
(In reply to David Cook from comment #8) > (In reply to Marcel de Rooy from comment #7) > > Without this patch, I have something like: > > > > [2024/08/02 08:58:38] [WARN] Opening /tmp/7PkHGkjawG//etc/selfie.jpg failed! > > at /usr/share/koha/tools/picture-upload.pl line 413. > > (Line number may be affected by a few debugs.) > > You haven't crafted your exploit quite right there. You'd need to preface > "/etc/selfie.jpg" with "../.." and then it would work. > Tried several variations. Retried this one: [2024/08/05 06:49:30] [WARN] Opening /tmp/ArJVJ8uzLT/../../etc/selfie.jpg failed! at /usr/share/koha/tools/picture-upload.pl line 416. Debian 11 container. There must be more to it..
(In reply to Marcel de Rooy from comment #9) > Tried several variations. Retried this one: > > [2024/08/05 06:49:30] [WARN] Opening /tmp/ArJVJ8uzLT/../../etc/selfie.jpg > failed! at /usr/share/koha/tools/picture-upload.pl line 416. > > Debian 11 container. > > There must be more to it.. It looks like you're not using ktd. I'm guessing it's with your work Koha setup? That could complicate things. Have you checked the file permissions? What permissions does the Koha user run as?
(In reply to David Cook from comment #10) > (In reply to Marcel de Rooy from comment #9) > > Tried several variations. Retried this one: > > > > [2024/08/05 06:49:30] [WARN] Opening /tmp/ArJVJ8uzLT/../../etc/selfie.jpg > > failed! at /usr/share/koha/tools/picture-upload.pl line 416. > > > > Debian 11 container. > > > > There must be more to it.. > > It looks like you're not using ktd. I'm guessing it's with your work Koha > setup? That could complicate things. Have you checked the file permissions? > What permissions does the Koha user run as? No ktd. But the setup is fine, running the koha user for Apache and Plack. I just realize that I was kind of misinterpreting your test plan and the contents of Nicks test zip file. We are not preventing to write to /tmp or /etc here! We are preventing to load from /tmp or /etc. So, all good.
Katrin, could you get this further please? Thx
(In reply to Marcel de Rooy from comment #12) > Katrin, could you get this further please? Thx Please check my note on the Mattermost security channel.
Pushed to main for 24.11. Nice work everyone, thanks!
Not backporting to 23.05.x unless requested