Bug 23451

Summary: Reflected XSS in opac-imageviewer.pl
Product: Koha Reporter: Daniel Kalinowski <daniel.kalinowski>
Component: OPACAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, katrin.fischer, lucas, martin.renvoize, nick, wizzyrea
Version: unspecified   
Hardware: All   
OS: All   
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
Bug 23451: Fix other similar wrong filterings
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl
Bug 23451: Fix other similar wrong filterings
Bug 23451: Prevent XSS vulnerabilities in opac-imageviewer.pl
Bug 23451: Fix other similar wrong filterings
Bug 23451: [18.11.x] Prevent XSS vulnerabilities in opac-imageviewer.pl
Bug 23451: [18.11.x] Fix other similar wrong filterings

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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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 (kidclamp) 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!