Bug 37146 - plugin_launcher.pl allows running of any Perl file on file system
Summary: plugin_launcher.pl allows running of any Perl file on file system
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-06-21 01:03 UTC by David Cook
Modified: 2024-08-01 15:43 UTC (History)
12 users (show)

See Also:
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 (1.92 KB, patch)
2024-06-21 01:51 UTC, David Cook
Details | Diff | Splinter Review
Bug 37146: Prevent path traversal by validating input (1.98 KB, patch)
2024-06-21 12:27 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 37146: Prevent path traversal by validating input (2.03 KB, patch)
2024-06-29 23:16 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 37146: Add tests (1.40 KB, patch)
2024-07-11 09:40 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!