acqui/booksellers.tt and admin/currency.tt use | $HtmlTags without first filtering the string they wrap.
Created attachment 177095 [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
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.
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.
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>
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
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.
Applied to 23.11.x-security branch
Applied to 22.11.x-security branch
(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 &? And if a translator uses &, did they mean &amp;? When those uses of $HtmlTags are required to be prefiltered, are they $raw or are they html?
Applied to 24.11.x-security
Pushed for 25.05! Well done everyone, thank you!