Description
Jonathan Druart
2016-08-02 14:08:27 UTC
Created attachment 53908 [details] [review] Bug 17023: Fix XSS in cataloguing/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Created attachment 53909 [details] [review] Bug 17023: Fix XSS in acqui/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Created attachment 53969 [details] [review] Bug 17023: Fix XSS in cataloguing/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 53970 [details] [review] Bug 17023: Fix XSS in acqui/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Created attachment 53975 [details] [review] [PASSED QA] Bug 17023: Fix XSS in cataloguing/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 53976 [details] [review] [PASSED QA] Bug 17023: Fix XSS in acqui/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to Master - Should be in the November 2016 release. Thanks! Pushed in 16.05. Will be in 16.05.03. Does not apply cleanly to 3.20.x Created attachment 54149 [details] [review] [3.20.x] Bug 17023: Fix XSS in cataloguing/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 54150 [details] [review] [3.20.x] Bug 17023: Fix XSS in acqui/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Thank you Jonathan, pushed to 3.20.x will be in 3.20.14 In 4th chunk of attachment 53976 [details] [review], [% basketno %] is changed into [% url %] I think it's not intended In attachment 54150 [details] [review] (for 3.20.x), it's corrected to [% basketno | url %] Also, why use 'url' filter here whereas the 'uri' filter is used everywhere else ? Created attachment 54533 [details] [review] [3.22.x] Bug 17023: Fix XSS in cataloguing/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> (cherry picked from commit f6e0a619d974fe002ec6a920206c7bc2808bce26) Signed-off-by: Frédéric Demians <f.demians@tamil.fr> (cherry picked from commit 98c6d7873535d52609657ecaf256740c95c0e532) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Created attachment 54534 [details] [review] [3.22.x] Bug 17023: Fix XSS in acqui/z3950_search.pl Test plan: Enter the following in the different inputs: <script>alert('XSS')</script> => Without this patch you will see the alert => With this patch, no more alert Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com> (cherry picked from commit eb543a90848b97d35aa15052c8881134926a3ed0) Signed-off-by: Frédéric Demians <f.demians@tamil.fr> (cherry picked from commit 7cb27f092a4c699fcd428083383eef6f515da3e3) Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> (In reply to Julian Maurice from comment #13) > In 4th chunk of attachment 53976 [details] [review] [review], > [% basketno %] is changed into [% url %] > I think it's not intended > > In attachment 54150 [details] [review] [review] (for 3.20.x), it's corrected to [% > basketno | url %] > > Also, why use 'url' filter here whereas the 'uri' filter is used everywhere > else ? It's a mistake, should be uri of course. Patches pushed to 3.22.x, will be in 3.22.10 Should I open a new bug to fix the "[% url %] => [% basketno | uri %]" problem in master ? (In reply to Julian Maurice from comment #17) > Patches pushed to 3.22.x, will be in 3.22.10 > > Should I open a new bug to fix the "[% url %] => [% basketno | uri %]" > problem in master ? It seems that it only appear in 3.20, Chris could you please patch it? (In reply to Jonathan Druart from comment #18) > (In reply to Julian Maurice from comment #17) > > Patches pushed to 3.22.x, will be in 3.22.10 > > > > Should I open a new bug to fix the "[% url %] => [% basketno | uri %]" > > problem in master ? > > It seems that it only appear in 3.20, Chris could you please patch it? In 3.20.x, it's [% basketno | url %], it could be fixed, but it's not a big issue IMO. In master, it's just [% url %] (line 111) Created attachment 54681 [details] [review] Bug 17023: Repair "cancel" link The basketno param has been removed in previous patch. (In reply to Jonathan Druart from comment #20) > Created attachment 54681 [details] [review] [review] > Bug 17023: Repair "cancel" link > > The basketno param has been removed in previous patch. Kyle, Frédéric and Julian, please push this regression fix. (Do not open a new bug report to make it available for stable releases). (In reply to Jonathan Druart from comment #21) > Kyle, Frédéric and Julian, please push this regression fix. Already fixed in 3.22.x Followup 'Bug 17023: Repair "cancel" link ' pushed to master! |