Bug 23451 - Reflected XSS in opac-imageviewer.pl
Summary: Reflected XSS in opac-imageviewer.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-12 11:38 UTC by Daniel Kalinowski
Modified: 2021-06-14 21:29 UTC (History)
7 users (show)

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


Attachments
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl (1.13 KB, patch)
2019-08-14 17:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23451: Fix other similar wrong filterings (16.01 KB, patch)
2019-08-14 17:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl (1.18 KB, patch)
2019-09-27 21:30 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 23451: Fix other similar wrong filterings (16.05 KB, patch)
2019-09-27 21:31 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl (1.23 KB, patch)
2019-09-30 11:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23451: Fix other similar wrong filterings (16.11 KB, patch)
2019-09-30 11:24 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23451: [18.11.x] Prevent XSS vulnerabilities in opac-imageviewer.pl (1.24 KB, patch)
2019-10-18 16:08 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23451: [18.11.x] Fix other similar wrong filterings (14.94 KB, patch)
2019-10-18 16:08 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Kalinowski 2019-08-12 11:38:54 UTC
File: koha-18.11.08/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc

  $("#ulactioncontainer > ul > li > a.addtoshelf").on("click",function(){
        Dopop('opac-addbybiblionumber.pl?biblionumber=[% biblionumber | html %]');
        return false;
    });
Vulnerable parameter: biblionumber||bib

PoC:
http://example/cgi-bin/koha/opac-imageviewer.pl?bib=x%27)})%3balert(1)%3b%20$(%27x%27).on(%27x%27,function(){x(%27

Note: Images have to be enabled for the catalog, otherwise bib||biblionumber wont be included in the page source code.
Comment 1 Jonathan Druart 2019-08-14 17:30:41 UTC
Confirmed on master.
Comment 2 Jonathan Druart 2019-08-14 17:41:01 UTC
Created attachment 92196 [details] [review]
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl

And certainly in other sripts as it is in opac-bottom.inc
Comment 3 Jonathan Druart 2019-08-14 17:41:05 UTC
Created attachment 92197 [details] [review]
Bug 23451: Fix other similar wrong filterings
Comment 4 Liz Rea 2019-09-27 21:30:51 UTC
Created attachment 93219 [details] [review]
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl

And certainly in other sripts as it is in opac-bottom.inc

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 5 Liz Rea 2019-09-27 21:31:00 UTC
Created attachment 93220 [details] [review]
Bug 23451: Fix other similar wrong filterings

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 6 Nick Clemens 2019-09-30 11:24:38 UTC
Created attachment 93232 [details] [review]
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl

And certainly in other sripts as it is in opac-bottom.inc

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens 2019-09-30 11:24:44 UTC
Created attachment 93233 [details] [review]
Bug 23451: Fix other similar wrong filterings

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Liz Rea 2019-10-17 17:35:13 UTC
I have applied this to 18.05.x, resolved some conflicts (mostly just putting the filters on what we were using on 18.05) and can release this at any time.
Comment 9 Lucas Gass 2019-10-18 15:29:41 UTC
cannot rebase this one for 18.11.x, can anyone help me out?
Comment 10 Nick Clemens 2019-10-18 16:08:46 UTC
Created attachment 94414 [details] [review]
Bug 23451: [18.11.x] Prevent XSS vulnerabilities in opac-imageviewer.pl

And certainly in other sripts as it is in opac-bottom.inc

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 11 Nick Clemens 2019-10-18 16:08:49 UTC
Created attachment 94415 [details] [review]
Bug 23451: [18.11.x] Fix other similar wrong filterings

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Martin Renvoize 2019-11-22 10:08:01 UTC
Comment on attachment 93233 [details] [review]
Bug 23451: Fix other similar wrong filterings

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

::: koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-results.tt
@@ +560,4 @@
>                                                          [% END # if TagsInputEnabled %]
>  
>                                                          [% IF ( ( Koha.Preference( 'opacuserlogin' ) == 1 ) && loggedinusername && ( Koha.Preference( 'virtualshelves' ) == 1 ) ) %]
> +                                                            <span class="actions"><a class="addtoshelf" href="/cgi-bin/koha/opac-addbybiblionumber.pl?biblionumber=[% SEARCH_RESULT.biblionumber | html %]" onclick="Dopop('opac-addbybiblionumber.pl?biblionumber=[% SEARCH_RESULT.biblionumber | uri %]'); return false;">Save to lists</a></span>

There's still a case of a html filter in a uri here.. was that deliberate?
Comment 13 Jonathan Druart 2019-11-22 10:11:50 UTC
Nope, must be uri as well!
Comment 14 Martin Renvoize 2019-11-22 10:12:23 UTC
Corrected the above on pushing to 19.05.x after a quick double check with Jonathan.
Comment 15 Martin Renvoize 2019-11-27 10:33:42 UTC
Nice work!

Pushed to master for 19.11.00
Comment 16 Daniel Kalinowski 2019-11-27 11:11:30 UTC
Great job!