Is this script used anymore? It needs a total rewrite to be made safe. I wonder if we can remove it instead?

The same information can be retrieved from the guided reports, without the sql injection problems. What do others think?

I suspect there are lots of libraries out there that use this script, it would be a pretty popular one for public libraries. 

Agree that we can get the same sort of data out of saved reports, but how best to make sure we don't lose the functionality in default Koha (aka "how do we not tick off the librarians")?

I'm not opposed to keeping the functionality but this script is a massive mess. I doubt I'll have time to rewrite it before Monday. So I'll probably remove it from 3.20 unless someone fixes it before then.

Maybe remove it and add a DB update that inserts an equivalent saved report? Question is, how to help people find it...

I think this script is in the GUI as 'Patrons who haven't checked out'.
The way it presents the data in a matrix can't be replicated by the guided reports/sql reports afaik. But the question is still if it is necessary for this kind of question. 

There are already sql reports in the library for similar questions. But I would not add one as default by update I think - as we can't write one that totally works the same.

Hm missed Liz comments at first... guess it's still too early for bug comments.

I think we have other reports having the same problem.
Not sure to remove them is the solution :)

I knew if I did a patch to remove it, someone would do one to fix it. :)

Created attachment 40502 [details] [review]
[PASSED QA] Bug 14426: Escape or use placeholders for sql parameters

Does this patch enough to prevent sql injection in borrowers_out.pl?

====================================================================
1. "Criteria" Parameter, Payload: ELT(1=1,'evil') / ELT(1=2,'evil')
====================================================================

echo -ne "POST /cgi-bin/koha/reports/borrowers_out.pl
HTTP/1.1\r\nHost: testbox:9002\r\nContent-Length:
186\r\n\r\nFilter=P_COM&Filter=&Limit=&output=file&basename=Export&MIME=CSV&sep=%3B&report_name=&do_it=1&userid=<username>&password=<password>&branch=&koha_login_context=intranet&Criteria=ELT(1=2,'evil')"
| nc testbox 9002

echo -ne "POST /cgi-bin/koha/reports/borrowers_out.pl
HTTP/1.1\r\nHost: testbox:9002\r\nContent-Length:
186\r\n\r\nFilter=P_COM&Filter=&Limit=&output=file&basename=Export&MIME=CSV&sep=%3B&report_name=&do_it=1&userid=<username>&password=<password>&branch=&koha_login_context=intranet&Criteria=ELT(1=1,'evil')"
| nc testbox 9002

====================================================================
2. "Filter" Parameter, Payload: P_COM'+AND+'a'='a / P_COM'+AND+'a'='b
====================================================================

echo -ne "POST /cgi-bin/koha/reports/borrowers_out.pl
HTTP/1.1\r\nHost: testbox:9002\r\nContent-Length:
183\r\n\r\nkoha_login_context=intranet&Limit=&Criteria=branchcode&output=file&basename=Export&MIME=CSV&sep=;&report_name=&do_it=1&userid=<userid>&password=<password>&branch=&Filter=P_COM'+AND+'a'='a"
| nc testbox 9002

echo -ne "POST /cgi-bin/koha/reports/borrowers_out.pl
HTTP/1.1\r\nHost: testbox:9002\r\nContent-Length:
183\r\n\r\nkoha_login_context=intranet&Limit=&Criteria=branchcode&output=file&basename=Export&MIME=CSV&sep=;&report_name=&do_it=1&userid=<userid>&password=<password>&branch=&Filter=P_COM'+AND+'a'='b"
| nc testbox 9002

====================================================================

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Pushed to 3.20.x will be in 3.20.1 (still needs to go into master too)

Pushed to 3.18.x will be in 3.18.08

Pushed to 3.14.x, will be in 3.14.16

Patch pushed to master.

Thanks Jonathan!

Pushed to 3.16.x, will be in 3.16.12