Bug 19333 - XSS vulnerability in opac-shelves
Summary: XSS vulnerability in opac-shelves
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
Depends on:
Reported: 2017-09-18 16:35 UTC by Tom HANSTRA
Modified: 2017-11-28 15:02 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:

Bug 19333: Fix XSS in opac-shelves (5.53 KB, patch)
2017-09-18 17:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 19333: Fix XSS in opac-shelves (5.62 KB, patch)
2017-10-11 12:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tom HANSTRA 2017-09-18 16:35:40 UTC
We received email from our campus InfoSec group that a portion of our Koha site was vulnerable to cross-site scripting attacks. Below is the gist of the email we received:

GET:  https://[our server]cgi-bin/koha/opac-shelves.pl?op=list&category=%22/%3E%3Cimg%20src=x%20onerror=%22alert(%27Doh!%20Insert%20Hax%20Here.%27)%22%20/%3E%3C!%E2%80%94

This page is vulnerable to Cross-site scripting attacks.

Cross-site scripting attacks, in general, are an issue because
they are enabling attacks. Specially-crafted malicious URLs can
steal authentication tokens/cookies when a logged-in user visits them,
giving the attacker full access to that user's account in the application.
Reflected XSS attacks, in particular, are a concern as they can be used to
socially engineer a user into clicking on what appears to be a legitimate URL.

Please also consider the following:

- Web application security testing should be performed regularly,
  especially for any public web applications. This includes
  tracking application inventory, general code review and vulnerability
  assessments using web application security testing tools.

- All input received by the web server should be checked before
  it is processed. The best method is to remove all unwanted input and
  accept only expected input. For example, ensure angle brackets are
  not allowed in any input to any Web page fields. Additionally, no
  syntactic input should be allowed. Syntactic input can come from
  databases, other servers, etc. All input into a Web application must
  be filtered to ensure the delivery of clean content to individuals using
  your service.

- Other References:

  OWASP Guide to Building Secure Web Applications and Web Services
Comment 1 Jonathan Druart 2017-09-18 17:54:50 UTC
Created attachment 67192 [details] [review]
Bug 19333: Fix XSS in opac-shelves

category is send back to the template, it must be escaped
Comment 2 M. Tompsett 2017-09-19 18:09:47 UTC
Are 506 and 514 correct?
What about line 685? 684 is escaped.
Comment 3 Jonathan Druart 2017-09-19 18:29:15 UTC
506                                             <input type="hidden" name="category" value="category" />
514                                             <input type="hidden" name="category" value="category" />

Are wrong, they are not TT variables

685                                                             <input type="hidden" name="category" value="[% s.category %]" />
Should be ok, come from the DB and is varchar(1)
Comment 4 Katrin Fischer 2017-10-02 23:57:15 UTC
Marking this failed QA because of Jonathan's comment#3
Comment 5 Jonathan Druart 2017-10-10 16:27:22 UTC
(In reply to Katrin Fischer from comment #4)
> Marking this failed QA because of Jonathan's comment#3

My comment was saying the patch is correct ;)
Comment 6 Marcel de Rooy 2017-10-11 12:05:28 UTC
Created attachment 67951 [details] [review]
Bug 19333: Fix XSS in opac-shelves

category is send back to the template, it must be escaped

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Marcel de Rooy 2017-10-11 12:05:52 UTC
Moving to PQA in same run
Comment 8 Jonathan Druart 2017-10-27 17:20:02 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 9 Katrin Fischer 2017-10-29 16:35:27 UTC
This patch has been pushed to 16.11.x and will be in 16.11.13.
Comment 10 Fridolin SOMERS 2017-11-08 08:22:14 UTC
These patches have been pushed to 17.05.x and are in 17.05.05.
Comment 11 Mason James 2017-11-23 23:18:06 UTC
Pushed to 16.05.x, for 16.05.18 release