XSS issue in basket.pl page
Amit is working on a patch, will be here tomorrow.
Amit, Is this still valid?
Hi Jonathan, Currently we are doing some security related testing for one of our client. I am planning to send all the patches to community. Please give us some time we will check and update the same.
Created attachment 65919 [details] [review] Bug 16069 - XSS issue in basket.pl page Used HTML::Entities - Encode or decode strings with HTML entities. ********************************************************* 1. Hit /cgi-bin/koha/acqui/basket.pl?basketno=xx<script>alert('amit')</script>. xx - is a basketno 2. Notice the java script is executed. 4. Apply patch. 5. Reload page, and hit the page again /cgi-bin/koha/acqui/basket.pl?basketno==xx<script>alert('amit')</script>. xx - is a basketno. 6. Notice it is no longer executed.
Created attachment 65931 [details] [review] Bug 16069 - XSS issue in basket.pl page 1. Hit /cgi-bin/koha/acqui/basket.pl?basketno=xx<script>alert('amit')</script>. xx - is a basketno 2. Notice the java script is executed. 4. Apply patch. 5. Reload page, and hit the page again /cgi-bin/koha/acqui/basket.pl?basketno==xx<script>alert('amit')</script>. xx - is a basketno. 6. Notice it is no longer executed. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 65940 [details] [review] Bug 16069 - XSS issue in basket.pl page 1. Hit /cgi-bin/koha/acqui/basket.pl?basketno=xx<script>alert('amit')</script>. xx - is a basketno 2. Notice the java script is executed. 4. Apply patch. 5. Reload page, and hit the page again /cgi-bin/koha/acqui/basket.pl?basketno==xx<script>alert('amit')</script>. xx - is a basketno. 6. Notice it is no longer executed. Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
On master I get: Can't call method "active" on an undefined value at /home/vagrant/kohaclone/acqui/basket.pl line 392. This is not exploitable. We should handle the case the basketno does not match a basket in DB instead.
Sorry, xx needs to be a valid basketno
I would prefer to stick to the current pattern (escape using the html filter in templates), but in this case it does not seem easy (lot of occurrences in the template). However it seems that at least booksellerid should be escaped as well.
Created attachment 66032 [details] [review] Bug 16069 - XSS issue in basket.pl page 1. Hit /cgi-bin/koha/acqui/basket.pl?basketno=xx<script>alert('amit')</script> xx - is a basketno 2. Notice the java script is executed. 3. Apply patch. 4. Reload page, and hit the page again /cgi-bin/koha/acqui/basket.pl?basketno==xx<script>alert('amit')</script> xx - is a basketno. 5. Notice it is no longer executed. ********************************** Thanks Jonathan for your suggestion. escape is done from the template.
Created attachment 66049 [details] [review] [SIGNED OFF] Bug 16069 - XSS issue in basket.pl page 1. Hit /cgi-bin/koha/acqui/basket.pl?basketno=xx<script>alert('amit')</script> xx - is a basketno 2. Notice the java script is executed. 3. Apply patch. 4. Reload page, and hit the page again /cgi-bin/koha/acqui/basket.pl?basketno==xx<script>alert('amit')</script> xx - is a basketno. 5. Notice it is no longer executed. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 66068 [details] [review] Bug 16069 - XSS issue in basket.pl page 1. Hit /cgi-bin/koha/acqui/basket.pl?basketno=xx<script>alert('amit')</script> xx - is a basketno 2. Notice the java script is executed. 3. Apply patch. 4. Reload page, and hit the page again /cgi-bin/koha/acqui/basket.pl?basketno==xx<script>alert('amit')</script> xx - is a basketno. 5. Notice it is no longer executed. Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
This doesn't apply cleanly to 16.11.x, some help appreciated: Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 16069 - XSS issue in basket.pl page fatal: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt). error: could not build fake ancestor Patch failed at 0001 Bug 16069 - XSS issue in basket.pl page The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-16069---XSS-issue-in-basketpl-page-bKPt_8.patch
Created attachment 66243 [details] [review] [16.11.x] Bug 16069 - XSS issue in basket.pl page Fixed for 16.11.x
Thx, worked perfectly!
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, is in 17.05.03
This patch has been pushed to 16.11.x and is in 16.11.11.
Pushed to 16.05.x, for 16.05.16 release - thanks Amit :0)