Bug 20982 - opac-shelves.pl vulnerable to Cross-site scripting attacks
Summary: opac-shelves.pl vulnerable to Cross-site scripting attacks
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All Linux
: P3 major (vote)
Assignee: Jonathan Druart
QA Contact: David Cook
URL:
Keywords:
: 28398 (view as bug list)
Depends on:
Blocks: 28409
  Show dependency treegraph
 
Reported: 2018-06-21 19:43 UTC by gbarna
Modified: 2021-05-26 07:52 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.06,20.05.12,19.11.18


Attachments
Bug 20982: Sanitize category to prevent XSS on opac-shelves.pl (1.44 KB, patch)
2021-05-20 06:36 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20982: Sanitize category to prevent XSS on opac-shelves.pl (1.82 KB, patch)
2021-05-20 19:17 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 20982: Sanitize category to prevent XSS on opac-shelves.pl (1.87 KB, patch)
2021-05-21 06:42 UTC, David Cook
Details | Diff | Splinter Review
Bug 20982: Comprehensively validate category in opac-shelves.pl (2.87 KB, patch)
2021-05-21 06:58 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description gbarna 2018-06-21 19:43:15 UTC
Our Information Security scanners have detected that opac-shelves.pl is vulnerable to Cross-site scripting attacks. We are currently on version 17.11.

GET:
https://catalog.uttyler.edu/cgi-bin/koha/opac-shelves.pl?op=list&category=2%22%3E%3Cscript%3Ealert(150)%3C/script%3E
Comment 1 Chris Cormack 2018-06-21 20:06:36 UTC
This was partially fixed in bug 19333

This error is coming from the paging (only can recreate when you have a list big enough to have multiple pages of results)
Comment 2 gbarna 2018-07-12 14:38:52 UTC
Any ETA on this bug? Our security scanner keeps flagging the vulnerability.
Comment 3 Jonathan Druart 2021-05-20 06:36:01 UTC
Created attachment 121209 [details] [review]
Bug 20982: Sanitize category to prevent XSS on opac-shelves.pl
Comment 4 Jonathan Druart 2021-05-20 07:32:55 UTC
*** Bug 28398 has been marked as a duplicate of this bug. ***
Comment 5 Victor Grousset/tuxayo 2021-05-20 19:17:49 UTC
Created attachment 121258 [details] [review]
Bug 20982: Sanitize category to prevent XSS on opac-shelves.pl

== Test plan ==
1. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}()
2. Note that you are redirected to another website
3. Apply the patch & restart services
4. Repeat the above and you are not redirected

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 6 Victor Grousset/tuxayo 2021-05-20 19:23:46 UTC
It seems to work :D
By testing with only the client and the server part, we can see that each of them is enough to fix. And more importantly that the server part is enough. So a compromised client can't bypass the fix.

Thanks David for the test plan.
Is that enough to test the vulnerability? I couldn't exploit with the example in this ticket.
Comment 7 David Cook 2021-05-21 06:13:31 UTC
Yeah looks good to me, Victor. I'll QA it shortly.
Comment 8 David Cook 2021-05-21 06:35:17 UTC
So there are other places where the category should be validated. However, I think we've done enough for this particular bug. I think that other validation is out of scope.
Comment 9 David Cook 2021-05-21 06:42:51 UTC
Created attachment 121264 [details] [review]
Bug 20982: Sanitize category to prevent XSS on opac-shelves.pl

== Test plan ==
1. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}()
2. Note that you are redirected to another website
3. Apply the patch & restart services
4. Repeat the above and you are not redirected

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 10 David Cook 2021-05-21 06:45:41 UTC
*** Bug 28409 has been marked as a duplicate of this bug. ***
Comment 11 David Cook 2021-05-21 06:45:57 UTC
Actually, it makes sense to validate it more comprehensively here...
Comment 12 David Cook 2021-05-21 06:58:39 UTC Comment hidden (obsolete)
Comment 13 Jonathan Druart 2021-05-21 07:08:30 UTC
Thanks for the follow-up, David.

I am reopening bug 28409 and moving your patch there. I have another follow-up and maybe more work is needed so I don't think it should be done here.
Comment 14 Victor Grousset/tuxayo 2021-05-21 15:01:57 UTC
So, passed QA for this one?
Comment 15 Victor Grousset/tuxayo 2021-05-22 13:46:23 UTC
Current version applies and work on 19.11.x
Comment 16 David Cook 2021-05-24 00:21:36 UTC
(In reply to Victor Grousset/tuxayo from comment #14)
> So, passed QA for this one?

Yep!
Comment 17 Victor Grousset/tuxayo 2021-05-24 19:09:10 UTC
> Yep!

Ok, so backported in my local secret 19.11.x branch ^^
Comment 18 Jonathan Druart 2021-05-26 07:50:32 UTC
Pushed to master for 21.05, thanks to everybody involved!