Bug 37146

Summary: plugin_launcher.pl allows running of any Perl file on file system
Product: Koha Reporter: David Cook <dcook>
Component: Plugin architectureAssignee: David Cook <dcook>
Status: Pushed to stable --- QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: dcook, didier.gautheron, fridolin.somers, jonathan.druart, laurent.ducos, lisette, lucas, m.de.rooy, martin.renvoize, nick, philippe.blouin, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
24.11.00,24.05.02,23.11.07,23.05.13,22.11.19
Version(s) released in:
Circulation function:
Attachments: Bug 37146: Prevent path traversal by validating input
Bug 37146: Prevent path traversal by validating input
Bug 37146: Prevent path traversal by validating input
Bug 37146: Add tests

Description David Cook 2024-06-21 01:03:14 UTC
Anyone with the "catalogue" permission (ie anyone with staff interface permission) can run any Perl file on the file system by using a maliciously crafted URL.
Comment 2 David Cook 2024-06-21 01:51:45 UTC
Created attachment 167944 [details] [review]
Bug 37146: Prevent path traversal by validating input

This patch validates the plugin_name passed to plugin_launcher.pl
against the base path containing the "value_builder" directory.

Test plan:
0. Apply the patch
1. koha-plack --reload kohadev
2. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=29
3. Check that the tag editor for leader still works
4. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
5. Check that the pluginf or "Date acquired" still works
Comment 3 David Cook 2024-06-21 01:58:24 UTC
Just noticed the unit test which I couldn't find before...
t/db_dependent/FrameworkPlugin.t

That's still passing which is good

--

I feel like I've heard of people providing their own framework plugins but hopefully they've been putting them in the right place and not relying on insecure relative paths...
Comment 5 David Cook 2024-06-21 02:17:15 UTC
(In reply to David Cook from comment #3)
> I feel like I've heard of people providing their own framework plugins but
> hopefully they've been putting them in the right place and not relying on
> insecure relative paths...

That said, if they've symlinked in their plugins from somewhere else, those won't work anymore...

I suppose an alternative to Cwd::abs_path() would be to use File::Basename to just get the actual filename and not any of the path. 

--

I'm not too fussed either way. Basically, we just need to validate that the user input refers to a file that's actually in the value_builder directory.
Comment 6 Nick Clemens (kidclamp) 2024-06-21 12:27:23 UTC
Created attachment 167957 [details] [review]
Bug 37146: Prevent path traversal by validating input

This patch validates the plugin_name passed to plugin_launcher.pl
against the base path containing the "value_builder" directory.

Test plan:
0. Apply the patch
1. koha-plack --reload kohadev
2. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=29
3. Check that the tag editor for leader still works
4. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
5. Check that the pluginf or "Date acquired" still works

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens (kidclamp) 2024-06-21 12:29:51 UTC
Just adding, the permissions here might need to be addressed - the launcher requires only 'catalogue' - the plugins themselves vary between:
catalogue => 1
editcatalogue => *

I do know that we have partners using cn_browser directly as a way to find items - s don't want to break that, however, raising above catalogue makes sense
Comment 8 Chris Cormack 2024-06-29 23:16:20 UTC
Created attachment 168301 [details] [review]
Bug 37146: Prevent path traversal by validating input

This patch validates the plugin_name passed to plugin_launcher.pl
against the base path containing the "value_builder" directory.

Test plan:
0. Apply the patch
1. koha-plack --reload kohadev
2. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?biblionumber=29
3. Check that the tag editor for leader still works
4. Go to http://localhost:8081/cgi-bin/koha/cataloguing/additem.pl?biblionumber=29
5. Check that the pluginf or "Date acquired" still works

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 9 Jonathan Druart 2024-07-11 09:40:49 UTC
Created attachment 168781 [details] [review]
Bug 37146: Add tests
Comment 10 Lucas Gass 2024-07-25 15:21:17 UTC
This has been backported to 24.05.x-security branch for 24.05.02
Comment 11 Katrin Fischer 2024-08-01 15:36:54 UTC
Pushed for 24.11!

Well done everyone, thank you!