XSS issue in supplier.pl page
Created attachment 65933 [details] [review] Bug 19089 - XSS issue in supplier.pl page Used HTML::Entities - Encode or decode strings with HTML entities. ****************************************************************** 1. Hit /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script>. xx - is a booksellerid 2. Notice the java script is executed. 4. Apply patch. 5. Reload page, and hit the page again /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script>. xx - is a booksellerid. 6. Notice it is no longer executed.
Created attachment 65934 [details] [review] Bug 19089 - XSS issue in supplier.pl page Used HTML::Entities - Encode or decode strings with HTML entities. ****************************************************************** 1. Hit /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script>. xx - is a booksellerid 2. Notice the java script is executed. 4. Apply patch. 5. Reload page, and hit the page again /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script>. xx - is a booksellerid. 6. Notice it is no longer executed.
Created attachment 65935 [details] [review] Bug 19089 - XSS issue in supplier.pl page 1. Hit /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script>. xx - is a booksellerid 2. Notice the java script is executed. 4. Apply patch. 5. Reload page, and hit the page again /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script>. xx - is a booksellerid. 6. Notice it is no longer executed. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> We probably should use a whitelist ie, it should only be digits. Rather than escaping it, but this works.
Hi Jonathan, Can you please add me on bug no 14568
We should stick to the pattern we use everywhere: escape from the template. Or, and better here, I would suggest to send the param only if a vendor has been found: my $booksellerid = $query->param('booksellerid'); my $supplier; if ($booksellerid) { $supplier = Koha::Acquisition::Booksellers->find( $booksellerid ); # Send the booksellerid vars to template and do other actions } Note that here, that could be done removing the following line: $template->{'VARS'}->{'booksellerid'} = $booksellerid; Second note: Currently it is not exploitable, if the $booksellerid does not correspond to a valid vendor, the ->unblessed statement will explode with Can't call method "unblessed" on an undefined value at /home/vagrant/kohaclone/acqui/supplier.pl line 72.
Created attachment 66030 [details] [review] Bug 19089 - XSS issue in supplier.pl page To Test 1. Hit /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script> xx - is a booksellerid 2. Notice the java script is executed. 3. Apply patch. 4. Reload page, and hit the page again /cgi-bin/koha/acqui/supplier.pl?booksellerid=xx<script>alert('amit')</script> xx - is a booksellerid. 5. Notice it is no longer executed. **************************************************************** Thanks Jonathan for your suggestion. escape is done from the template
(In reply to Jonathan Druart from comment #5) > Or, and better here, I would suggest to send the param only if a vendor has > been found: > > my $booksellerid = $query->param('booksellerid'); > my $supplier; > if ($booksellerid) { > $supplier = Koha::Acquisition::Booksellers->find( $booksellerid ); > # Send the booksellerid vars to template and do other actions > } > > > Note that here, that could be done removing the following line: > $template->{'VARS'}->{'booksellerid'} = $booksellerid; > > Second note: Currently it is not exploitable, if the $booksellerid does not > correspond to a valid vendor, the ->unblessed statement will explode with > Can't call method "unblessed" on an undefined value at > /home/vagrant/kohaclone/acqui/supplier.pl line 72. => For the record: all of that was wrong: > select count(*) from aqbooksellers where id="1<script>alert('XSS')</script>"; returns 1.
I can't reproduce the error here: /cgi-bin/koha/acqui/supplier.pl?booksellerid=5<SCRIPT>alert("Test")</SCRIPT> Results in bad display, but the alert is not executed.
Hm, tried again - still no luck in getting the alert executed.
Tried again, please take another look.
This has been solved by or prior bug 13618.