Bug 38961

Summary: XSS in vendor search
Product: Koha Reporter: Phil Ringnalda <phil>
Component: AcquisitionsAssignee: Phil Ringnalda <phil>
Status: RESOLVED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: major    
Priority: P3 CC: alexbuckley, dcook, didier.gautheron, fridolin.somers, jesse, jonathan.druart, katrin.fischer, laurent.ducos, lisette, lucas, m.de.rooy, magnus, martin.renvoize, nick, paul.derscheid, philippe.blouin, tomascohen
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00,24.11.02,24.05.07,23.11.12,22.11.24
Circulation function:
Attachments: Bug 38961: XSS in vendor search
Bug 38961: XSS in vendor search
Bug 38961: XSS in vendor search
Bug 38961: XSS in vendor search

Description Phil Ringnalda 2025-01-24 01:47:23 UTC
acqui/booksellers.tt and admin/currency.tt use | $HtmlTags without first filtering the string they wrap.
Comment 2 Phil Ringnalda 2025-01-24 02:05:43 UTC Comment hidden (obsolete)
Comment 4 Magnus Enger 2025-01-24 07:24:02 UTC
Created attachment 177097 [details] [review]
Bug 38961: XSS in vendor search

acqui/booksellers.tt and admin/currency.tt use | $HtmlTags without first
filtering the string they wrap.

Test plan:
1. Without the patch, load the URL in comment 1, note the alert()
2. Apply patch, load the URL in comment 1, no alert()

Sponsored-by: Chetco Community Public Library
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan, works as advertised.
Comment 6 Jonathan Druart 2025-01-27 10:08:42 UTC
Created attachment 177171 [details] [review]
Bug 38961: XSS in vendor search

acqui/booksellers.tt and admin/currency.tt use | $HtmlTags without first
filtering the string they wrap.

Test plan:
1. Without the patch, load the URL in comment 1, note the alert()
2. Apply patch, load the URL in comment 1, no alert()

Sponsored-by: Chetco Community Public Library
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan, works as advertised.
Comment 7 Jonathan Druart 2025-01-27 10:09:04 UTC
Created attachment 177172 [details] [review]
Bug 38961: XSS in vendor search

acqui/booksellers.tt and admin/currency.tt use | $HtmlTags without first
filtering the string they wrap.

Test plan:
1. Without the patch, load the URL in comment 1, note the alert()
2. Apply patch, load the URL in comment 1, no alert()

Sponsored-by: Chetco Community Public Library
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Followed the test plan, works as advertised.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Alex Buckley 2025-02-05 02:18:45 UTC
Applied to 24.05.x-security branch.

We had to amend this patch slightly to be able to apply it on 24.05

The conflict was:

$ sed -n '/<<<<</,/>>>>>/p' koha-tmpl/intranet-tmpl/prog/en/modules/admin/currency.tt
<<<<<<< HEAD
    <div class="dialog alert">
        <h1>Cannot delete currency [% currency.currency | $HtmlTags tag='span' attributes=>'class="ex"' %]</h1>
=======
    <div class="alert alert-warning">
        <h1>Cannot delete currency [% currency.currency | html | $HtmlTags tag='span' attributes=>'class="ex"' %]</h1>
>>>>>>> Bug 38961: XSS in vendor search


We changed it to:

    <div class="dialog alert">
        <h1>Cannot delete currency [% currency.currency | html | $HtmlTags tag='span' attributes=>'class="ex"' %]</h1>
        <p>

Here is our patch on the 24.05.x-security branch: https://git.koha-community.org/Koha-community/security/commit/a5afd9c9c8bd3901597a45998bf42b257cf91690
Comment 12 Fridolin Somers 2025-02-14 14:17:32 UTC
Looks like the filter is missing in koha-tmpl/intranet-tmpl/prog/en/modules/installer/step2.tt

<li>Database [% dbname | $HtmlTags tag=>'code' %] exists.</li>

Other places use html or a static string.
Comment 13 Fridolin Somers 2025-02-14 14:20:34 UTC
Applied to 23.11.x-security branch
Comment 14 Jesse Maseto 2025-02-18 20:57:55 UTC
Applied to 22.11.x-security branch
Comment 15 Phil Ringnalda 2025-02-20 21:36:28 UTC
(In reply to Fridolin Somers from comment #12)
> Looks like the filter is missing in
> koha-tmpl/intranet-tmpl/prog/en/modules/installer/step2.tt
> 
> <li>Database [% dbname | $HtmlTags tag=>'code' %] exists.</li>
> 
> Other places use html or a static string.

True, those should be html filtered since "<script>alert('Robert\'); DROP TABLE Students;-- ');</script>" is a valid (if pathological) MySQL username. But as far as I can tell, those all come directly from koha-conf.xml, and if you've lost control over it you've lost the game. So rather than a security bug, they'll have to wait for the one about requiring a prefilter on every use of $HtmlTags.

The static strings are actually more interesting, since I presume they are translated strings (that being the point of $HtmlTags). If a translator uses an ampersand in a string, did they mean &, or did they mean &amp;? And if a translator uses &amp;, did they mean &amp;amp;? When those uses of $HtmlTags are required to be prefiltered, are they $raw or are they html?
Comment 16 Paul Derscheid 2025-02-22 18:16:20 UTC
Applied to 24.11.x-security
Comment 17 Katrin Fischer 2025-03-06 13:00:29 UTC
Pushed for 25.05!

Well done everyone, thank you!