Hi, yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-1 will result in an Error: Can't call method "raw" on an undefined value at /usr/share/koha/lib/C4/Search.pm line 490. yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-999999999999999999 will also result in an Error: Can't call method "raw" on an undefined value at /usr/share/koha/lib/C4/Search.pm line 490. but try yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 I added one 9, and you won't have a response. *Seems to crash, lock overloaded (have no idea) the Koha or DB process. For most website running Koha, it will crash it, if not, it's likely that either DB or Koha is using multithread. Then just run the payload for the number of thread your DB or Koh is using and you will down the website. *I'm guessing with 0 knowledge of Koha, I just want to watch another Netflix TV-Show and don't have the courage to look at the code, or to run a Koha instance. Forgive me. Kuddos. I tried that on some of the test websites there https://koha-community.org/demo/ so they might be down :/ sorry.
Ok, I was a bit scared to not see the koha instance I tried getting back up so I set up my own instance. So the search.pl process will get locked at 100%, and also the apache2 process. What was a bit scary was that lot of data were written on my disk. So 30 sec after running the query I had a koha error file of 1.5GB filled with : [Sun Jun 25 00:49:13.475921 2017] [cgi:error] [pid XXX] [client XXXXXXX] AH01215: [Sun Jun 25 00:49:13 2017] search.pl: ERROR DECODING RECORD - Empty String at /XXXXXX/XXXXXX/perl5/MARC/File/XML.pm line 448.: /XXXXXX/XXXXXX/koha/intranet/cgi-bin/catalogue/search.pl A bit scary.
Thank you for reporting this, we'll hopefully have a fix shortly.
Created attachment 64611 [details] [review] Bug 18854 - Protect from DOS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client
Created attachment 64612 [details] [review] Bug 18854 - Protect from DOS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Created attachment 64617 [details] [review] Bug 18854 - Protect from DOS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Amended: changed -2 to 0 in opac-search.pl.
I think this bug should also apply on opac/opac-showreviews.pl $offset. No ?
If they use zebra then yes
(In reply to Fridolin SOMERS from comment #6) > I think this bug should also apply on opac/opac-showreviews.pl $offset. > No ? In fact no, looks like it is already protected : my $offset = $query->param('offset') || 0; my $page = $query->param('page') || 1; $offset = ($page-1)*$results_per_page if $page>1; Sorry its all good.
Hmm that could still end up negative, i think safest probably to do it there too.
opac-showreviews generates internal server error on large negative offsets, no dos problems afaics
Hi guys, would might be cool to write a small fuzzer for KOHA in order to detect this kind of stuff in many more, it would then be easy to integrate in the CI. Would love to do it but right now don't have the time.
IMO this check must be done in Koha::SearchEngine::[Elasticsearch|Zebra]::Search::search_compat to cover all calls.
Created attachment 64799 [details] [review] [ALTERNATIVE-PATCH] Bug 18854: Make sure offset will not be < 0 - protect from DoS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client
QA, please provide feedback very quickly.
(In reply to Jonathan Druart from comment #14) > QA, please provide feedback very quickly. I agree the test could be moved into Search.pm.
Created attachment 64800 [details] [review] Bug 18854: Make sure offset will not be < 0 - protect from DoS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
(In reply to Tomás Cohen Arazi from comment #15) > (In reply to Jonathan Druart from comment #14) > > QA, please provide feedback very quickly. > > I agree the test could be moved into Search.pm. I think both patches are useful here, because $offset is also used at other places in catalogue/search.pl and opac/opac-search.pl QAing this now
Created attachment 64888 [details] [review] Bug 18854 - Protect from DOS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Amended: changed -2 to 0 in opac-search.pl. Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 64889 [details] [review] Bug 18854: Make sure offset will not be < 0 - protect from DoS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Created attachment 64969 [details] [review] Bug 18854: Protect few other occurrences of offset
Can I get a QA stamp on the last patch?
Created attachment 65039 [details] [review] Bug 18854: Make sure offset will not be < 0 - protect from DoS There was a bug that meant a very large offset in the search params will cause the search script to run forever (or long enough to crash the machine) To test 1/ Get ready with sudo top so you can kill the thread before it causes your machine to OOM 2/ Hit a page like yourdomain.com/cgi-bin/koha/opac-search.pl?q=1&offset=-9999999999999999999 3/ Notice the process runs for a long time 4/ Kill the process 5/ Apply the patch 6/ Hit the page again, notice the it loads (offset is set to zero) 7/ Do the same to search in the staff client Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Julian Maurice <julian.maurice@biblibre.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 65040 [details] [review] Bug 18854: Protect few other occurrences of offset Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 17.11, thanks to everybody involved!
Pushed to 17.05.x, will be in 17.05.02
These patches have been pushed to 16.11.x and will be in 16.11.10.