[Bug 22836] New: Tests catching XSS vulnerabilities in pagination are not correct
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Bug ID: 22836 Summary: Tests catching XSS vulnerabilities in pagination are not correct Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: Test Suite Assignee: jonathan.druart@bugs.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org QA Contact: testopia@bugs.koha-community.org Target Milestone: --- See bug 22478 comments 44 and 45. The tests were added originally to catch XSS vulnerabilities when pagination was used (shelves, reviews, authorities searches, etc.). With one of the QA followup (Handle category in opac-shelves like a boolean) we did not trust the escape by resetting the "category" if not set to 1 or 2. We should rely on the correct filtering instead. However, if one really wants to use this change, we should adapt the tests to catch the correct filtered values (and so do not use unlike), in another area (i.e. not shelves, where we are handling the invalid values differently). I am suggestion to revert those patches, as it is the easiest solution. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |22478 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22478 [Bug 22478] Cross-site scripting vulnerability in paginations -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #1 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 89264 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89264&action=edit Revert "Bug 22478: (QA follow-up) Update tests to check for any script tags" This reverts commit d9b3b90849fd95d296b73b330c4e32ff018c81a5. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 89265 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89265&action=edit Revert "Bug 22478: (QA follow-up) Handle category in opac-shelves like a boolean" This reverts commit 375dd35d5354d46c4c16f534494b123ce370c3f4. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #3 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 89266 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89266&action=edit Bug 22836: (follow-up) Bug 22478: (QA follow-up) Make test consistent with variable name One occurrence was missing. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #4 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #2)
Created attachment 89265 [details] [review] Revert "Bug 22478: (QA follow-up) Handle category in opac-shelves like a boolean"
This reverts commit 375dd35d5354d46c4c16f534494b123ce370c3f4.
I don't see the sense of removing these lines. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #5 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Hello Marcel, I have tried to explain it in the comment of this bug report. Which part does not make sense to you? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #6 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Jonathan Druart from comment #5)
Hello Marcel, I have tried to explain it in the comment of this bug report. Which part does not make sense to you?
Improve filtering is great. But we do not need to remove solid code. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #7 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Marcel de Rooy from comment #6)
(In reply to Jonathan Druart from comment #5)
Hello Marcel, I have tried to explain it in the comment of this bug report. Which part does not make sense to you?
Improve filtering is great. But we do not need to remove solid code.
Then we need to write more tests. I *strongly* think that we must remove this code. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- IMO It's not solid code, it's code which leads us to believe that the values are not correctly filtered. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |major Keywords| |rel_19_05_candidate -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |martin.renvoize@ptfs-europe | |.com --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I've come to the conclusion that Jonathan is correct here. 1) We need to have a generic test for missing filters in pagination 2) The 'noise' created by not cleaning up the lines directly is inconsequential (in so much as it'll only ever appear if someone is attempting to XSS us AND the end user will never see it unless they're inspecting the html). Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89264|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 89677 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89677&action=edit Revert "Bug 22478: (QA follow-up) Update tests to check for any script tags" This reverts commit d9b3b90849fd95d296b73b330c4e32ff018c81a5. https://bugs.koha-community.org/show_bug.cgi?id=22836 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89265|0 |1 is obsolete| | --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 89678 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89678&action=edit Revert "Bug 22478: (QA follow-up) Handle category in opac-shelves like a boolean" This reverts commit 375dd35d5354d46c4c16f534494b123ce370c3f4. https://bugs.koha-community.org/show_bug.cgi?id=22836 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89266|0 |1 is obsolete| | --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 89679 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89679&action=edit Bug 22836: (follow-up) Bug 22478: (QA follow-up) Make test consistent with variable name One occurrence was missing. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89677|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 89710 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89710&action=edit Revert "Bug 22478: (QA follow-up) Update tests to check for any script tags" This reverts commit d9b3b90849fd95d296b73b330c4e32ff018c81a5. https://bugs.koha-community.org/show_bug.cgi?id=22836 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89678|0 |1 is obsolete| | --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 89711 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89711&action=edit Revert "Bug 22478: (QA follow-up) Handle category in opac-shelves like a boolean" This reverts commit 375dd35d5354d46c4c16f534494b123ce370c3f4. https://bugs.koha-community.org/show_bug.cgi?id=22836 Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #89679|0 |1 is obsolete| | --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 89712 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=89712&action=edit Bug 22836: (follow-up) Bug 22478: (QA follow-up) Make test consistent with variable name One occurrence was missing. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #16 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I am not sure about the commit messages - the first 2 are not referring to this bug. Leaving that to Nick ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Katrin Fischer from comment #16)
I am not sure about the commit messages - the first 2 are not referring to this bug. Leaving that to Nick ;)
They are ok, as they are reverting commits I think we should keep this syntax. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com Status|Passed QA |Pushed to Master --- Comment #18 from Nick Clemens <nick@bywatersolutions.com> --- Awesome work all! Pushed to master for 19.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |19.05.00, 18.11.06 released in| | Status|Pushed to Master |Pushed to Stable --- Comment #19 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Pushed to 18.11.x for 18.11.06 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22836 Bug 22836 depends on bug 22478, which changed state. Bug 22478 Summary: Cross-site scripting vulnerability in paginations https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22478 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org