Description
David Cook
2021-05-21 06:41:28 UTC
Actually changed my mind... going to just update 20982... *** This bug has been marked as a duplicate of bug 20982 *** Created attachment 121267 [details] [review] Bug 28409: Comprehensively validate category in opac-shelves.pl Default to a category of 1 (ie Private). Only allow input of 1 or 2 (ie Public) == Test plan == 1. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}() 2. Note that you are redirected to another website 3. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=add&shelfname=foo&category=9 4. Note that you can't see this list in the Lists (but it has been added to the database) 5. Apply the patch & restart services 6. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}() 7. Note that you are not redirected to another website 8. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=add&shelfname=bar&category=9 9. Note that "bar" has been added as a Private list Created attachment 121268 [details] [review] Bug 28409: Simplify data validation Simplify the affectation then trust it. Then shouldn't we apply the same in shelves.pl (staff)? Created attachment 121281 [details] [review] Bug 28409: Comprehensively validate category in opac-shelves.pl Default to a category of 1 (ie Private). Only allow input of 1 or 2 (ie Public) == Test plan == 1. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}() 2. Note that you are redirected to another website 3. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=add&shelfname=foo&category=9 4. Note that you can't see this list in the Lists (but it has been added to the database) 5. Apply the patch & restart services 6. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}() 7. Note that you are not redirected to another website 8. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=add&shelfname=bar&category=9 9. Note that "bar" has been added as a Private list Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Created attachment 121282 [details] [review] Bug 28409: Simplify data validation Simplify the affectation then trust it. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> It work! :D Hum, why is step 1 the same as bug 20982 ? The fix of it also fixes this case. Current version applies and work on 19.11.x Comment on attachment 121282 [details] [review] Bug 28409: Simplify data validation Review of attachment 121282 [details] [review]: ----------------------------------------------------------------- ::: opac/opac-shelves.pl @@ +109,4 @@ > $shelf = Koha::Virtualshelf->new( > { shelfname => scalar $query->param('shelfname'), > sortfield => scalar $query->param('sortfield'), > + category => $category,, Little syntax error here Created attachment 121308 [details] [review] Bug 28409: Simplify data validation Simplify the affectation then trust it. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> JD Amended patch: remove duplicate comma Created attachment 121487 [details] [review] Bug 28409: Comprehensively validate category in opac-shelves.pl Default to a category of 1 (ie Private). Only allow input of 1 or 2 (ie Public) == Test plan == 1. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}() 2. Note that you are redirected to another website 3. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=add&shelfname=foo&category=9 4. Note that you can't see this list in the Lists (but it has been added to the database) 5. Apply the patch & restart services 6. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?category=function(){window.location.href%20=%20%27https://git.koha-community.org/stats/koha-master/authors.html%27}() 7. Note that you are not redirected to another website 8. Go to http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=add&shelfname=bar&category=9 9. Note that "bar" has been added as a Private list Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 121488 [details] [review] Bug 28409: Simplify data validation Simplify the affectation then trust it. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> JD Amended patch: remove duplicate comma Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Ready for 20.05.x, pushed to the security branch. It works. Note comment 10, this hasn't changed so only the 2nd test case was reproducible and fixed with the patch. The 1st test case already worked. Trouble! (also in master) After running the full test suite on 20.05.x-security, there is this failure: It's doesn't fail without these patches. But with them: t/db_dependent/selenium/regressions.t .. 4/6 Use of uninitialized value $charge in multiplication (*) at /kohadevbox/koha/C4/Circulation.pm line 3267. t/db_dependent/selenium/regressions.t .. 5/6 # Failed test 'The second page should display the variables and attributes correctly URI escaped' # at t/db_dependent/selenium/regressions.t line 257. # 'http://koha:8080/cgi-bin/koha/opac-shelves.pl?page=2&category=2&op=list' # doesn't match '(?^u:category=2%22%3E%3Cscript%3Ealert%28%27booh%21%27%29%3C%2Fscript%3E)' # Looks like you failed 1 test of 3. t/db_dependent/selenium/regressions.t .. 6/6 # Failed test 'XSS vulnerabilities in pagination' # at t/db_dependent/selenium/regressions.t line 262. # Looks like you failed 1 test of 6. t/db_dependent/selenium/regressions.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/6 subtests ============ master ================= It's doesn't fail without these patches. But with them: t/db_dependent/selenium/regressions.t .. 4/7 item-level_itypes set but no itemtype set for item (973) at /kohadevbox/koha/Koha/Schema/Result/Item.pm line 877. item-level_itypes set but no itemtype set for item (973) at /kohadevbox/koha/Koha/Schema/Result/Item.pm line 877. Use of uninitialized value $charge in multiplication (*) at /kohadevbox/koha/C4/Circulation.pm line 3394. item-level_itypes set but no itemtype set for item (973) at /kohadevbox/koha/Koha/Schema/Result/Item.pm line 877. item-level_itypes set but no itemtype set for item (973) at /kohadevbox/koha/Koha/Schema/Result/Item.pm line 877. item-level_itypes set but no itemtype set for item (973) at /kohadevbox/koha/Koha/Schema/Result/Item.pm line 877. t/db_dependent/selenium/regressions.t .. 5/7 # Failed test 'The second page should display the variables and attributes correctly URI escaped' # at t/db_dependent/selenium/regressions.t line 254. # 'http://koha:8080/cgi-bin/koha/opac-shelves.pl?page=2&op=list&category=2' # doesn't match '(?^u:category=2%22%3E%3Cscript%3Ealert%28%27booh%21%27%29%3C%2Fscript%3E)' # Looks like you failed 1 test of 3. t/db_dependent/selenium/regressions.t .. 6/7 # Failed test 'XSS vulnerabilities in pagination' # at t/db_dependent/selenium/regressions.t line 258. Wide character in print at /usr/share/perl/5.32/Test2/Formatter/TAP.pm line 156. Wide character in print at /usr/share/perl/5.32/Test2/Formatter/TAP.pm line 156. Wide character in print at /usr/share/perl/5.32/Test2/Formatter/TAP.pm line 156. Wide character in print at /usr/share/perl/5.32/Test2/Formatter/TAP.pm line 156. Wide character in print at /usr/share/perl/5.32/Test2/Formatter/TAP.pm line 156. Wide character in print at /usr/share/perl/5.32/Test2/Formatter/TAP.pm line 156. t/db_dependent/selenium/regressions.t .. 7/7 # Looks like you failed 1 test of 7. t/db_dependent/selenium/regressions.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/7 subtests ====== unrelated side note ======= master compared to 20.05.x has more warnings. It that worth opening a ticket? item-level_itypes set but no itemtype set for item (975) at /kohadevbox/koha/Koha/Schema/Result/Item.pm line 877. Wide character in print at /usr/share/perl/5.32/Test2/Formatter/TAP.pm line 156. (In reply to Victor Grousset/tuxayo from comment #18) > Trouble! (also in master) Nothing to worry, it's just the test that is no longer valid. Created attachment 122358 [details] [review] Bug 28409: Adjust regression test We are no longer expecting an URI escaped value but a corrected category value, either 1 or 2. Created attachment 122359 [details] [review] Bug 28409: Adjust regression test We are no longer expecting an URI escaped value but a corrected category value, either 1 or 2. It works, thanks! :D Backported to 19.11.x for 19.11.19 Backported: Pushed to 20.05.x branch for 20.05.13 Backported: Pushed to 20.11.x branch for 20.11.07 Pushed to master for 21.11, thanks to everybody involved! |