Attachment 69656 Details for Bug 19319 – Bug 19319: Reflected XSS Vulnerability in opac-MARCdetail.pl

[patch] Bug 19319: Reflected XSS Vulnerability in opac-MARCdetail.pl

Bug-19319-Reflected-XSS-Vulnerability-in-opac-MARC.patch (text/plain), 20.74 KB, created by Jonathan Druart on 2017-12-08 18:03:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-12-08 18:03:50 UTC

Size: 20.74 KB

patch

obsolete

>From 07ab89dea959232e877f6c504692ad4eef809f2d Mon Sep 17 00:00:00 2001
>From: Kyle M Hall <kyle@bywatersolutions.com>
>Date: Thu, 14 Sep 2017 11:52:08 -0400
>Subject: [PATCH] Bug 19319: Reflected XSS Vulnerability in opac-MARCdetail.pl
>
>Try going to this URL on your site: /cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2"><TEST>
>
>Test Plan:
>1) Go to /cgi-bin/koha/opac-MARCdetail.pl?biblionumber=2"><TEST>
>2) Note <TEST> is embedded all over the html
>3) Apply this patch
>4) Refresh the page, note the injection is gone!
>5) run koha qa test tools
>
>Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
>
>Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
>
>Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>---
> .../bootstrap/en/includes/opac-detail-sidebar.inc  | 14 ++++-----
> .../bootstrap/en/modules/opac-ISBDdetail.tt        |  2 +-
> .../bootstrap/en/modules/opac-MARCdetail.tt        | 12 ++++----
> .../opac-tmpl/bootstrap/en/modules/opac-detail.tt  | 34 +++++++++++-----------
> opac/opac-ISBDdetail.pl                            |  6 ++--
> opac/opac-MARCdetail.pl                            |  2 +-
> opac/opac-detail.pl                                |  4 +--
> 7 files changed, 38 insertions(+), 36 deletions(-)
>
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc
>index 5e96b0f6bf..7ceaa9590e 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-detail-sidebar.inc
>@@ -4,7 +4,7 @@
>         [% IF Koha.Preference( 'opacuserlogin' ) == 1 %]
>             [% IF Koha.Preference( 'RequestOnOpac' ) == 1 %]
>                 [% IF ( AllowOnShelfHolds OR ItemsIssued ) %]
>-                    <li><a class="reserve" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% biblionumber | html %]">Place hold</a></li>
>+                    <li><a class="reserve" href="/cgi-bin/koha/opac-reserve.pl?biblionumber=[% biblio.biblionumber %]">Place hold</a></li>
>                 [% END %]
>             [% END %]
>         [% END %]
>@@ -14,21 +14,21 @@
> 
>     [% IF Koha.Preference( 'opacuserlogin' ) == 1 %]
>         [% IF Koha.Preference('ArticleRequests') %]
>-            <li><a class="article_request" href="/cgi-bin/koha/opac-request-article.pl?biblionumber=[% biblionumber | html %]">Request article</a></li>
>+            <li><a class="article_request" href="/cgi-bin/koha/opac-request-article.pl?biblionumber=[% biblio.biblionumber %]">Request article</a></li>
>         [% END %]
>     [% END %]
> 
>     [% IF Koha.Preference( 'virtualshelves' ) == 1 %]
>         [% IF ( ( Koha.Preference( 'opacuserlogin' ) == 1 ) && loggedinusername ) %]
>-            <li><a class="addtoshelf" href="/cgi-bin/koha/opac-addbybiblionumber.pl?biblionumber=[% biblionumber | html %]">Save to your lists</a></li>
>+            <li><a class="addtoshelf" href="/cgi-bin/koha/opac-addbybiblionumber.pl?biblionumber=[% biblio.biblionumber %]">Save to your lists</a></li>
>         [% END %]
>     [% END %]
> 
>     [% IF Koha.Preference( 'opacbookbag' ) == 1 %]
>         [% IF ( incart ) %]
>-            <li><a class="incart cart[% biblionumber | html %] addrecord" href="#">In your cart</a> <a class="cartRemove cartR[% biblionumber | html %]" href="#">(remove)</a></li>
>+            <li><a class="incart cart[% biblio.biblionumber %] addrecord" href="#">In your cart</a> <a class="cartRemove cartR[% biblio.biblionumber %]" href="#">(remove)</a></li>
>         [% ELSE %]
>-            <li><a class="addtocart cart[% biblionumber | html %] addrecord" href="#">Add to your cart</a>  <a style="display:none;" class="cartRemove cartR[% biblionumber | html %]" href="#">(remove)</a></li>
>+            <li><a class="addtocart cart[% biblio.biblionumber %] addrecord" href="#">Add to your cart</a>  <a style="display:none;" class="cartRemove cartR[% biblio.biblionumber %]" href="#">(remove)</a></li>
>         [% END %]
>     [% END %]
> 
>@@ -51,7 +51,7 @@
>                                     <li><a role="menuitem" href="#" data-toggle="modal" data-target="#exportModal_">Dublin Core</a></li>
>                                 [% ELSE %]
>                                 <li>
>-                                    <a role="menuitem" href="/cgi-bin/koha/opac-export.pl?op=export&amp;bib=[% biblionumber | html %]&amp;format=[% option %]">
>+                                    <a role="menuitem" href="/cgi-bin/koha/opac-export.pl?op=export&amp;bib=[% biblio.biblionumber %]&amp;format=[% option %]">
>                                         [% SWITCH option %]
>                                             [% CASE 'bibtex' %]BIBTEX
>                                             [% CASE 'endnote' %]EndNote
>@@ -107,7 +107,7 @@
>                 <label class="label_dc" for="input-srw">SRW-DC</label>
>                 <br>
>         <input type="hidden" name="op" value="export">
>-        <input type="hidden" name="bib" value="[% biblionumber | html %]">
>+        <input type="hidden" name="bib" value="[% biblio.biblionumber %]">
>         </fieldset>
>     </div>
>     <div class="modal-footer">
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-ISBDdetail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-ISBDdetail.tt
>index 42d5f79dc0..8371680631 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-ISBDdetail.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-ISBDdetail.tt
>@@ -19,7 +19,7 @@
>                     <div id="usermarcdetail">
>                         <div id="catalogue_detail_biblio">
>                             <div id="views">
>-                                <span class="view"><a id="Normalview" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% biblionumber | html %]">Normal view</a></span> <span class="view"><a id="MARCview" href="/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=[% biblionumber | html %]">MARC view</a></span> <span class="view current-view"><span id="ISBDview">ISBD view</span></span></div>
>+                                <span class="view"><a id="Normalview" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% biblio.biblionumber %]">Normal view</a></span> <span class="view"><a id="MARCview" href="/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=[% biblio.biblionumber %]">MARC view</a></span> <span class="view current-view"><span id="ISBDview">ISBD view</span></span></div>
> 
>                                 <div id="isbdcontents">[% ISBD %]</div>
> 
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-MARCdetail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-MARCdetail.tt
>index 7b0f9849a5..f230639963 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-MARCdetail.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-MARCdetail.tt
>@@ -1,6 +1,6 @@
> [% USE Koha %]
> [% INCLUDE 'doc-head-open.inc' %]
>-<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog &rsaquo;  MARC details for record no. [% biblionumber | html %]</title>
>+<title>[% IF ( LibraryNameTitle ) %][% LibraryNameTitle %][% ELSE %]Koha online[% END %] catalog &rsaquo;  MARC details for record no. [% biblio.biblionumber %]</title>
> [% INCLUDE 'doc-head-close.inc' %]
> [% BLOCK cssinclude %][% END %]
> </head>
>@@ -20,14 +20,14 @@
>                         <div id="catalogue_detail_biblio">
> 
>                             <div id="views">
>-                                <span class="view"><a id="Normalview" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% biblionumber | html %]">Normal view</a></span>
>+                                <span class="view"><a id="Normalview" href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% biblio.biblionumber %]">Normal view</a></span>
>                                 <span class="view current-view"><span id="MARCview">MARC view</span></span>
>-                                [% IF ( ISBD ) %]<span class="view"><a id="ISBDview"  href="/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=[% biblionumber | html %]">ISBD view</a></span>[% END %]
>+                                [% IF ( ISBD ) %]<span class="view"><a id="ISBDview"  href="/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=[% biblio.biblionumber %]">ISBD view</a></span>[% END %]
>                             </div>
>-                            <h1 class="title">[% bibliotitle %] (Record no. [% biblionumber | html %])</h1>
>+                            <h1 class="title">[% bibliotitle %] (Record no. [% biblio.biblionumber %])</h1>
> 
>                             [% IF ( OPACXSLTDetailsDisplay ) %]
>-                                <div id="switchview_div">[ <a id="switchview" href="/cgi-bin/koha/opac-showmarc.pl?id=[% biblionumber | html %]&amp;viewas=html">view plain</a> ]</div>
>+                                <div id="switchview_div">[ <a id="switchview" href="/cgi-bin/koha/opac-showmarc.pl?id=[% biblio.biblionumber %]&amp;viewas=html">view plain</a> ]</div>
>                                 <div id="plainmarc"></div>
>                             [% END %]
> 
>@@ -194,7 +194,7 @@ $(document).ready(function(){
>             $(this).text(_("view labeled"));
>             $("#labeledmarc").hide();
>             if(!loaded){
>-                $("#plainmarc").show().html("<div style=\"margin:1em;padding:1em;border:1px solid #EEE;font-size:150%;\"><img src=\"[% interface %]/[% theme %]/images/loading.gif\" /> "+_("Loading")+"...</div>").load("/cgi-bin/koha/opac-showmarc.pl","id=[% biblionumber | html %]&viewas=html");
>+                $("#plainmarc").show().html("<div style=\"margin:1em;padding:1em;border:1px solid #EEE;font-size:150%;\"><img src=\"[% interface %]/[% theme %]/images/loading.gif\" /> "+_("Loading")+"...</div>").load("/cgi-bin/koha/opac-showmarc.pl","id=[% biblio.biblionumber %]&viewas=html");
>                 loaded = 1;
>             } else {
>                 $("#plainmarc").show();
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>index eb857ecffb..61479d3ad5 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-detail.tt
>@@ -57,7 +57,7 @@
> 
>                     <div id="bookcover">
>                     [% IF ( OPACLocalCoverImages ) %]
>-                        <div title="[% biblionumber |url %]" class="[% biblionumber | html %]" id="local-thumbnail-preview"></div>
>+                        <div title="[% biblionumber |url %]" class="[% biblio.biblionumber %]" id="local-thumbnail-preview"></div>
>                     [% END %]
>                     [% IF ( OPACAmazonCoverImages ) %]
>                         [% IF ( OPACURLOpenInNewWindow ) %]
>@@ -105,15 +105,15 @@
>                     [% END %]
>                     </div>
> 
>-                    <abbr class="unapi-id" title="koha:biblionumber:[% biblionumber | html %]"></abbr>
>+                    <abbr class="unapi-id" title="koha:biblionumber:[% biblio.biblionumber %]"></abbr>
>                     [% IF ( ocoins ) # COinS / Openurl %]
>                         <span class="Z3988" title="[% ocoins %]"></span>
>                     [% END %]
> 
>                     <div id="views">
>                         <span class="view current-view"><span id="Normalview">Normal view</span></span>
>-                        <span class="view"><a id="MARCview" href="/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=[% biblionumber | html %]">MARC view</a></span>
>-                        [% IF ( ISBD ) %]<span class="view"><a id="ISBDview" href="/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=[% biblionumber | html %]">ISBD view</a></span>[% END %]
>+                        <span class="view"><a id="MARCview" href="/cgi-bin/koha/opac-MARCdetail.pl?biblionumber=[% biblio.biblionumber %]">MARC view</a></span>
>+                        [% IF ( ISBD ) %]<span class="view"><a id="ISBDview" href="/cgi-bin/koha/opac-ISBDdetail.pl?biblionumber=[% biblio.biblionumber %]">ISBD view</a></span>[% END %]
>                     </div>
>                     [% IF ( OPACXSLTDetailsDisplay ) %]
>                         [% XSLTBloc %]
>@@ -409,16 +409,16 @@
>                             [% END %]
>                             [% IF ( TagsInputEnabled ) %]
>                                 [% IF ( loggedinusername ) %]
>-                                    <form id="tagform[% biblionumber | html %]" method="post" action="/cgi-bin/koha/opac-tags.pl" style="display:none;">
>-                                        <label for="newtag[% biblionumber | html %]">New tag(s), separated by a comma:</label>
>-                                        <input name="newtag[% biblionumber | html %]" id="newtag[% biblionumber | html %]" maxlength="100" type="text"/>
>-                                        <input name="tagbutton" class="btn btn-small tagbutton" title="[% biblionumber | html %]" type="submit" value="Add" />
>-                                        <a class="cancel_tag_add" id="cancel[% biblionumber | html %]" href="#">(done)</a>
>+                                    <form id="tagform[% biblio.biblionumber %]" method="post" action="/cgi-bin/koha/opac-tags.pl" style="display:none;">
>+                                        <label for="newtag[% biblio.biblionumber %]">New tag(s), separated by a comma:</label>
>+                                        <input name="newtag[% biblio.biblionumber %]" id="newtag[% biblio.biblionumber %]" maxlength="100" type="text"/>
>+                                        <input name="tagbutton" class="btn btn-small tagbutton" title="[% biblio.biblionumber %]" type="submit" value="Add" />
>+                                        <a class="cancel_tag_add" id="cancel[% biblio.biblionumber %]" href="#">(done)</a>
>                                     </form>
>-                                    <span id="newtag[% biblionumber | html %]_status" class="tagstatus" style="display:none;">
>+                                    <span id="newtag[% biblio.biblionumber %]_status" class="tagstatus" style="display:none;">
>                                         Tag status here.
>                                     </span>
>-                                    <a class="tag_add" id="tag_add[% biblionumber | html %]" href="#">Add tag(s)</a>
>+                                    <a class="tag_add" id="tag_add[% biblio.biblionumber %]" href="#">Add tag(s)</a>
>                                 [% ELSE %]
>                                     <span id="login4tags">
>                                         [% IF Koha.Preference('casAuthentication') %]
>@@ -498,7 +498,7 @@
> 
>                                 
> 
>-                                <input  type="hidden" name='biblionumber'  value="[% biblionumber | html %]" />
>+                                <input  type="hidden" name='biblionumber'  value="[% biblio.biblionumber %]" />
>                                 <input  type="hidden" name='rating_value' id='rating_value' value="[% my_rating.rating_value %]" />
> 
>                                 [% UNLESS ( rating_readonly ) %]&nbsp;  <input name="rate_button" type="submit" value="Rate me" />[% END %]&nbsp;
>@@ -652,7 +652,7 @@
> 
>                     <div id="holdings">
>                         [% IF too_many_items %]
>-                            <p>This record has many physical items ([% items_count %]). <a href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% biblionumber | html %]&amp;viewallitems=1">Click here to view them all.</a></p>
>+                            <p>This record has many physical items ([% items_count %]). <a href="/cgi-bin/koha/opac-detail.pl?biblionumber=[% biblio.biblionumber %]&amp;viewallitems=1">Click here to view them all.</a></p>
>                         [% ELSIF ( itemloop.size ) %]
>                             [% INCLUDE items_table items=itemloop tab="holdings" table_id="holdingst" %]
>                             [% IF Koha.Preference('OPACAcquisitionDetails') and acquisition_details.total_quantity > 0 %]
>@@ -875,7 +875,7 @@
>                                     </table>
>                                 [% END # / IF subscription.latestserials %]
>                             [% END # / FOREACH subscriptions %]
>-                            <p class="subscription_moredetails"><a href="opac-serial-issues.pl?biblionumber=[% biblionumber | html %]">More details</a></p>
>+                            <p class="subscription_moredetails"><a href="opac-serial-issues.pl?biblionumber=[% biblio.biblionumber %]">More details</a></p>
>                         </div> 
>                     [% END # IF subscriptionsnumber %]
> 
>@@ -959,7 +959,7 @@
> 
>                                 [% IF ( loggedinusername ) %]
>                                     [% UNLESS ( loggedincommenter ) %]
>-                                       <div id="addcomment"> <a href="#" onclick="Dopop('/cgi-bin/koha/opac-review.pl?biblionumber=[% biblionumber | html %]'); return false;">
>+                                       <div id="addcomment"> <a href="#" onclick="Dopop('/cgi-bin/koha/opac-review.pl?biblionumber=[% biblio.biblionumber %]'); return false;">
>                                             Post your comments on this item.
>                                         </a></div>
>                                     [% END %]
>@@ -1032,7 +1032,7 @@
>                                 <p>Click on an image to view it in the image viewer</p>
>                                 [% FOREACH image IN localimages %]
>                                     [% IF image %]
>-                                        <a class="localimage" href="/cgi-bin/koha/opac-imageviewer.pl?biblionumber=[% biblionumber | html %]&amp;imagenumber=[% image %]"><img alt="" src="/cgi-bin/koha/opac-image.pl?thumbnail=1&amp;imagenumber=[% image %]" /></a>
>+                                        <a class="localimage" href="/cgi-bin/koha/opac-imageviewer.pl?biblionumber=[% biblio.biblionumber %]&amp;imagenumber=[% image %]"><img alt="" src="/cgi-bin/koha/opac-image.pl?thumbnail=1&amp;imagenumber=[% image %]" /></a>
>                                     [% END %]
>                                 [% END %]
>                             </div>
>@@ -1648,7 +1648,7 @@
>               $.post("/cgi-bin/koha/opac-ratings-ajax.pl", {
>                 rating_old_value: $("#rating_value").attr("value"),
>                 borrowernumber: "[% borrowernumber %]",
>-                biblionumber: "[% biblionumber | html %]",
>+                biblionumber: "[% biblio.biblionumber %]",
>                 rating_value: value,
>                 auth_error: value
>               }, function (data) {
>diff --git a/opac/opac-ISBDdetail.pl b/opac/opac-ISBDdetail.pl
>index a3cb1a9705..7898e80609 100755
>--- a/opac/opac-ISBDdetail.pl
>+++ b/opac/opac-ISBDdetail.pl
>@@ -55,7 +55,7 @@ use C4::Koha;
> use Koha::ItemTypes;
> use Koha::Patrons;
> use Koha::RecordProcessor;
>-
>+use Koha::Biblios;
> 
> my $query = CGI->new();
> my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
>@@ -71,6 +71,8 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user(
> my $biblionumber = $query->param('biblionumber');
> $biblionumber = int($biblionumber);
> 
>+my $biblio = Koha::Biblios->find( $biblionumber );
>+
> # get biblionumbers stored in the cart
> if(my $cart_list = $query->cookie("bib_list")){
>     my @cart_list = split(/\//, $cart_list);
>@@ -185,7 +187,7 @@ $template->param(
>     AllowOnShelfHolds   => $allow_onshelf_holds,
>     norequests   => $norequests,
>     ISBD         => $res,
>-    biblionumber => $biblionumber,
>+    biblio       => $biblio,
> );
> 
> #Search for title in links
>diff --git a/opac/opac-MARCdetail.pl b/opac/opac-MARCdetail.pl
>index 9605cdd7e6..334e383e25 100755
>--- a/opac/opac-MARCdetail.pl
>+++ b/opac/opac-MARCdetail.pl
>@@ -349,7 +349,7 @@ $template->param(
>     item_loop           => \@item_loop,
>     item_header_loop    => \@item_header_loop,
>     item_subfield_codes => \@item_subfield_codes,
>-    biblionumber        => $biblionumber,
>+    biblio              => $biblio,
> );
> 
> output_html_with_http_headers $query, $cookie, $template->output;
>diff --git a/opac/opac-detail.pl b/opac/opac-detail.pl
>index 819e42692f..29f371e152 100755
>--- a/opac/opac-detail.pl
>+++ b/opac/opac-detail.pl
>@@ -78,6 +78,7 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user(
> 
> my $biblionumber = $query->param('biblionumber') || $query->param('bib') || 0;
> $biblionumber = int($biblionumber);
>+my $biblio = Koha::Biblios->find( $biblionumber );
> 
> my @all_items = GetItemsInfo($biblionumber);
> my @hiddenitems;
>@@ -136,7 +137,7 @@ if (C4::Context->preference('OpacSuppression')) {
>     }
> }
> 
>-$template->param( biblionumber => $biblionumber );
>+$template->param( biblio => $biblio );
> 
> # get biblionumbers stored in the cart
> my @cart_list;
>@@ -603,7 +604,6 @@ for ( C4::Context->preference("OPACShowHoldQueueDetails") ) {
> }
> my $has_hold;
> if ( $show_holds_count || $show_priority) {
>-    my $biblio = Koha::Biblios->find( $biblionumber );
>     my $holds = $biblio->holds;
>     $template->param( holds_count  => $holds->count );
>     while ( my $hold = $holds->next ) {
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 19319: 67140 | 68482 | 68486 | 69513 | 69634 | 69635 | 69656 | 69657 | 69658