Bug 13425 - There is an xss vulnerability in 3.16, 3.18 and master in the opac and intranet facets
Summary: There is an xss vulnerability in 3.16, 3.18 and master in the opac and intran...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: 3.18
Hardware: All All
: P5 - low critical (vote)
Assignee: Galen Charlton
QA Contact: Testopia
URL:
Keywords:
: 13426 (view as bug list)
Depends on:
Blocks: 13428
  Show dependency treegraph
 
Reported: 2014-12-09 23:47 UTC by Chris Cormack
Modified: 2019-06-27 09:24 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 13425 - XSS in opac facets - Patch for master and 3.18 (2.12 KB, patch)
2014-12-09 23:49 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13425 - XSS in opac facets - Patch for 3.16 (3.11 KB, patch)
2014-12-09 23:51 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13425 - XSS in opac facets - Patch for 3.16 (1.57 KB, patch)
2014-12-10 00:03 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 13425 - XSS in opac facets - Patch for 3.16 (1.57 KB, patch)
2014-12-10 00:04 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 13425 - XSS in opac facets - Patch for 3.16 (1.57 KB, patch)
2014-12-10 00:05 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 13425 - XSS in opac facets - Patch for master and 3.18 (2.19 KB, patch)
2014-12-10 00:05 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 13425 - XSS in opac facets - Patch for 3.16 (1.57 KB, patch)
2014-12-10 00:05 UTC, Brendan Gallagher
Details | Diff | Splinter Review
Bug 13425 - XSS in opac facets - Patch for master and 3.18 (2.29 KB, patch)
2014-12-10 00:07 UTC, Mirko Tietgen
Details | Diff | Splinter Review
[PUSHED] Bug 13425 - XSS in intranet facets - Patch for 3.16 (1.52 KB, patch)
2014-12-10 04:42 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
[PUSHED] Bug 13425 - XSS in opac facets - Patch for 3.16 (3.18 KB, patch)
2014-12-10 04:43 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 13425: Fix regression on deleting facets (1.17 KB, patch)
2014-12-10 08:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
[PUSHED] Bug 13425 - XSS in opac facets - Patch for master and 3.18 (2.36 KB, patch)
2014-12-10 09:17 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (1.53 KB, patch)
2014-12-14 23:09 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (1.83 KB, patch)
2014-12-17 20:08 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (1.89 KB, patch)
2014-12-18 08:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (3.02 KB, patch)
2014-12-19 23:13 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (3.13 KB, patch)
2014-12-21 20:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (5.99 KB, patch)
2014-12-21 20:50 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (6.16 KB, patch)
2014-12-21 20:56 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master (5.95 KB, patch)
2014-12-22 17:02 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Cormack 2014-12-09 23:47:14 UTC
A url like 
/cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123 


Will cause the js to be executed. THere is a missing escaped url.
Comment 1 Chris Cormack 2014-12-09 23:49:01 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2014-12-09 23:51:11 UTC Comment hidden (obsolete)
Comment 6 Brendan Gallagher 2014-12-10 00:05:30 UTC Comment hidden (obsolete)
Comment 7 Brendan Gallagher 2014-12-10 00:05:35 UTC Comment hidden (obsolete)
Comment 8 Brendan Gallagher 2014-12-10 00:06:48 UTC
tested and works.  Sorry I kept hitting the wrong button - hence why I've got the same patch signed off 10 times.  :P
Comment 9 Mirko Tietgen 2014-12-10 00:07:47 UTC Comment hidden (obsolete)
Comment 10 Chris Cormack 2014-12-10 01:30:29 UTC
Pushed to 3.18.x still needs to go to master and 3.16.x
Comment 11 Chris Cormack 2014-12-10 01:31:01 UTC
*** Bug 13426 has been marked as a duplicate of this bug. ***
Comment 12 Jared Camins-Esakov 2014-12-10 04:42:49 UTC
Created attachment 34264 [details] [review]
[PUSHED] Bug 13425 - XSS in intranet facets - Patch for 3.16

To Test
1/ Craft a url like /cgi-bin/koha/catalogue/search.pl?q=smith&sort_by='"><script>prompt('Happy_Holidays')</script>

It is important it must return results and facets

2/ Notice the js is executed
3/ Apply the patch test again

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 13 Jared Camins-Esakov 2014-12-10 04:43:01 UTC
Created attachment 34265 [details] [review]
[PUSHED] Bug 13425 - XSS in opac facets - Patch for 3.16

To Test
1/ Craft a url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123

It is important it must return results and facets

2/ Notice the js is executed
3/ Apply the patch test again

Test this one both in prog and bootstrap please

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Comment 14 Jared Camins-Esakov 2014-12-10 04:43:37 UTC
Brendan, I removed your sign-off line because it looks like you may have uploaded a different signed off patch than the one you were testing. Please feel free to readd it.
Comment 15 Jonathan Druart 2014-12-10 08:22:43 UTC
The patch fixes the issues, but introduce a regression:
Search for "123", click on a facet with a diacritic (eg. "Bazin, Hervé").
Remove this facet using the [x] link.
You cannot remove the facet.
This worked before the patch.
Comment 16 Chris Cormack 2014-12-10 08:29:10 UTC
All the patch does is escape the html in the url, if this breaks the functionality of the remove facet, then that code needs to be reworked, we simply can't have a feature that relies on us displaying unfiltered user input.
Comment 17 Chris Cormack 2014-12-10 08:31:29 UTC
Especially when the code to exploit it has been published :(
Comment 18 Jonathan Druart 2014-12-10 08:39:32 UTC Comment hidden (obsolete)
Comment 19 Chris Cormack 2014-12-10 08:42:28 UTC
I think that new patch should go on its own bug, outside of the security group so it can get signed off and qaed as normal.

And can go out in the 3.18.2 release (and 3.16.6), and tomas can push to master when he is back.

Do you want me to shift it?
Comment 20 Jonathan Druart 2014-12-10 09:00:44 UTC
(In reply to Jonathan Druart from comment #18)
> Created attachment 34267 [details] [review] [review]

moved to bug 13428.
Comment 22 Jonathan Druart 2014-12-10 09:17:45 UTC
Created attachment 34273 [details] [review]
[PUSHED] Bug 13425 - XSS in opac facets - Patch for master and 3.18

To Test
1/ Craft a url like /cgi-bin/koha/opac-search.pl?q=123&sort_by='"><script>prompt('Happy_Holidays')</script>&limit=123

It is important it must return results and facets

2/ Notice the js is executed
3/ Apply the patch test again

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Popup is gone after applying the patch. Facet link still shows it but does not execute. It's gone after clicking the link.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
This patch needs bug 13428.
Comment 23 Chris Cormack 2014-12-10 09:20:18 UTC
Hmm there are 2 patches for 3.16.x one for intranet one for opac, mason must have missed the opac one. I dont think he has rolled the release or done an announcement yet, so he should be able to fix that.
Comment 24 Katrin Fischer 2014-12-11 10:14:07 UTC
I can confirm that OPAC in 3.16 is NOT fixed. I just tested it. The confusing bit is that the commit message claims to fix the OPAC, but indeed it fixes the intranet :(

http://git.koha-community.org/gitweb/?p=koha.git;a=commit;h=30ca6c1d0eaf15ffc4339cb20595241182b9cdae
Comment 25 Katrin Fischer 2014-12-11 10:18:52 UTC
On another note: I'd like to receive emails about changes on the security bugs - is there something I can do? I thought there was a security mailing list too, but can't remember receiving an email.
Comment 26 Katrin Fischer 2014-12-11 11:21:30 UTC
Comment on attachment 34265 [details] [review]
[PUSHED] Bug 13425 - XSS in opac facets - Patch for 3.16

Review of attachment 34265 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-facets.inc
@@ +42,4 @@
>                                    [% IF facet.active %]
>                                      [% SET url = url _ "&amp;nolimit=" _ facet.type_link_value _ ":" _ facet.facet_link_value %]
>                                      <span class="facet-label">[% facet.facet_label_value %]</span>
> +                                    [<a href="[% url |url%]" title="Remove facet [% facet.facet_link_value | html %]">x</a>]

This change seems to only appear in this patch - not in the one for bootstrap or intranet - is it intentional?
It seems to break the functionality, but we are still running tests.
Comment 27 Jonathan Druart 2014-12-11 11:32:10 UTC
I completely forgot to test the prog theme...
Comment 28 Mason James 2014-12-11 12:54:42 UTC
(In reply to Chris Cormack from comment #23)
> Hmm there are 2 patches for 3.16.x one for intranet one for opac, mason must
> have missed the opac one. I dont think he has rolled the release or done an
> announcement yet, so he should be able to fix that.

the 2nd missing patch has now been pushed to 3.16 branch
Comment 29 Mason James 2014-12-11 23:27:33 UTC
(In reply to Jonathan Druart from comment #27)
> I completely forgot to test the prog theme...


I just tested the PROG theme too

the bug is fixed :)
Comment 30 Chris Cormack 2014-12-14 23:09:32 UTC Comment hidden (obsolete)
Comment 31 Chris Cormack 2014-12-14 23:10:03 UTC
Just the last patch for the intranet needs to be tested
Comment 32 Chris Cormack 2014-12-17 20:08:37 UTC Comment hidden (obsolete)
Comment 33 Jonathan Druart 2014-12-18 08:18:30 UTC Comment hidden (obsolete)
Comment 34 Jonathan Druart 2014-12-18 08:19:56 UTC
The last patch is pushed to 3.18 and should be pushed to master.
Marked as Passed QA.
Comment 35 Chris Cormack 2014-12-18 08:23:49 UTC
It isn't actually pushed to 3.18 yet, I was wanting a sign off on it. But if you have tested it, ill push it to 3.18 as soon as it's in master.
Comment 36 Jonathan Druart 2014-12-18 09:34:56 UTC
(In reply to Chris Cormack from comment #35)
> It isn't actually pushed to 3.18 yet, I was wanting a sign off on it. But if
> you have tested it, ill push it to 3.18 as soon as it's in master.

Arg yes, sorry to add confusion.

So the last patch needs to be pushed to 3.18.x and master.
Comment 37 Chris Cormack 2014-12-19 23:13:14 UTC Comment hidden (obsolete)
Comment 38 Chris Cormack 2014-12-19 23:13:58 UTC
Reworked the last patch, it was missing the fix for the availability facet, it needs new testing please
Comment 39 Katrin Fischer 2014-12-21 20:43:57 UTC Comment hidden (obsolete)
Comment 40 Chris Cormack 2014-12-21 20:50:40 UTC Comment hidden (obsolete)
Comment 41 Katrin Fischer 2014-12-21 20:56:24 UTC Comment hidden (obsolete)
Comment 42 Chris Cormack 2014-12-21 21:00:52 UTC
Pushed to 3.18.x, Tomas just the last patch needs to go to master, at your convenience  :)
Comment 43 Mason James 2014-12-22 17:02:11 UTC
Created attachment 34636 [details] [review]
Bug 13425 - XSS in intranet facets - Patch for 3.18 and master

To Test
1/ Craft a url like /cgi-bin/koha/catalogue/search.pl?q=smith&sort_by='"><script>prompt('Happy_Holidays')</script>

It is important it must return results and facets

2/ Notice the js is executed
3/ Apply the patch test again

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
No prompts, no functional regressions found.
Checked selecting and undoing facets, show more links and paging.
Signed-off-by: Mason James <mtj@kohaaloha.com>
Comment 44 Mason James 2014-12-22 17:05:30 UTC
(In reply to Chris Cormack from comment #42)
> Pushed to 3.18.x, Tomas just the last patch needs to go to master, at your
> convenience  :)

This last patch looks to fix a CGI quoting bug in 3.16.x

I plan to include this patch in 3.16.6
Comment 45 Chris Cormack 2014-12-26 19:41:11 UTC
Tomas the last intranet patch needs to be pushed to master, and then we can move this out of security into the open, and I will request some cve numbers.
Comment 46 Chris Cormack 2015-01-20 20:33:14 UTC
We got this assigned a CVE number CVE-2014-9446


http://www.cvedetails.com/cve/CVE-2014-9446/
Comment 47 Fridolin Somers 2015-02-23 07:51:29 UTC
Not needed for 3.14.x, I set has resolved.