Bug 35290 - SQL Injection vulnerability in ysearch.pl
Summary: SQL Injection vulnerability in ysearch.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: David Cook
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on:
Blocks: 35835
  Show dependency treegraph
 
Reported: 2023-11-08 15:08 UTC by Ryan Henderson
Modified: 2024-09-05 07:02 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.06,22.11.12,22.05.17,21.11.26
Circulation function:


Attachments
Bug 35290: Sanitize field input on cataloguing/ysearch.pl (1.49 KB, patch)
2023-11-08 23:46 UTC, David Cook
Details | Diff | Splinter Review
Bug 35290: Sanitize field input on cataloguing/ysearch.pl (1.54 KB, patch)
2023-11-09 19:49 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35290: (follow-up) Perltidy formatting corrections (1.08 KB, patch)
2023-11-09 19:49 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35290: Sanitize field input on cataloguing/ysearch.pl (1.59 KB, patch)
2023-11-09 20:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 35290: (follow-up) Perltidy formatting corrections (1.14 KB, patch)
2023-11-09 20:28 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Henderson 2023-11-08 15:08:01 UTC
Hello community, 

We've recently had a penetration test run on one of our servers and it returned a vulnerability within the ysearch.pl file. 

As you can see from the following this was discovered within 23.05.04 but is also present within 22.11.10:

https://nvd.nist.gov/vuln/detail/CVE-2023-44961

The following GitHub explains the PoC for this vulnerability: 

https://github.com/ggb0n/CVE-2023-44961
Comment 1 Jonathan Druart 2023-11-08 15:41:24 UTC
Can easily be fixed as it's only used in a couple of places:

cataloguing/value_builder/marc21_field_260b.pl:            source: '/cgi-bin/koha/cataloguing/ysearch.pl?table=biblioitems&field=publishercode',
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc_field_210c_bis.tt:                source: "/cgi-bin/koha/cataloguing/ysearch.pl?table=biblioitems&field=publishercode",
koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/unimarc_field_225a_bis.tt:                source: "/cgi-bin/koha/cataloguing/ysearch.pl?table=biblioitems&field=collectiontitle",

Sanitizing the input should be trivial.
Comment 2 David Cook 2023-11-08 22:42:02 UTC
(In reply to Jonathan Druart from comment #1)
> Can easily be fixed as it's only used in a couple of places:
> 
> cataloguing/value_builder/marc21_field_260b.pl:            source:
> '/cgi-bin/koha/cataloguing/ysearch.pl?table=biblioitems&field=publishercode',
> koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/
> unimarc_field_210c_bis.tt:                source:
> "/cgi-bin/koha/cataloguing/ysearch.pl?table=biblioitems&field=publishercode",
> koha-tmpl/intranet-tmpl/prog/en/modules/cataloguing/value_builder/
> unimarc_field_225a_bis.tt:                source:
> "/cgi-bin/koha/cataloguing/ysearch.
> pl?table=biblioitems&field=collectiontitle",
> 
> Sanitizing the input should be trivial.

Looks like we're already filtering "table", so "field" just needs to be filtered as well for "publishercode" or "collectiontitle" it seems?
Comment 3 David Cook 2023-11-08 23:46:23 UTC
Created attachment 158682 [details] [review]
Bug 35290: Sanitize field input on cataloguing/ysearch.pl

This change sanitizies the field input on cataloguing/ysearch.pl

Test plan:
0. Apply the patch and restart/reload Koha
1a. "Add marc21_field_260b.pl plugin to 260$b in the Default framework"
1b. Go to http://localhost:8081/cgi-bin/koha/admin/marc_subfields_structure.pl?
op=add_form&tagfield=260&frameworkcode=#subbfield
1c. Choose "marc21_field_260b.pl" from the dropdown next to "Plugin"
1d. Click "Save changes"
2a. "Add new record"
2b. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?frameworkcode=
3. Click on tab "2" and scroll down to 260 "b"
4. Type in "Ori" into 260 subfield b
5. Some autocomplete suggestions should appear
Comment 4 Owen Leonard 2023-11-09 19:49:54 UTC
Created attachment 158762 [details] [review]
Bug 35290: Sanitize field input on cataloguing/ysearch.pl

This change sanitizies the field input on cataloguing/ysearch.pl

Test plan:
0. Apply the patch and restart/reload Koha
1a. "Add marc21_field_260b.pl plugin to 260$b in the Default framework"
1b. Go to http://localhost:8081/cgi-bin/koha/admin/marc_subfields_structure.pl?
op=add_form&tagfield=260&frameworkcode=#subbfield
1c. Choose "marc21_field_260b.pl" from the dropdown next to "Plugin"
1d. Click "Save changes"
2a. "Add new record"
2b. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?frameworkcode=
3. Click on tab "2" and scroll down to 260 "b"
4. Type in "Ori" into 260 subfield b
5. Some autocomplete suggestions should appear

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Owen Leonard 2023-11-09 19:49:57 UTC
Created attachment 158763 [details] [review]
Bug 35290: (follow-up) Perltidy formatting corrections
Comment 6 Nick Clemens (kidclamp) 2023-11-09 20:28:52 UTC
Created attachment 158764 [details] [review]
Bug 35290: Sanitize field input on cataloguing/ysearch.pl

This change sanitizies the field input on cataloguing/ysearch.pl

Test plan:
0. Apply the patch and restart/reload Koha
1a. "Add marc21_field_260b.pl plugin to 260$b in the Default framework"
1b. Go to http://localhost:8081/cgi-bin/koha/admin/marc_subfields_structure.pl?
op=add_form&tagfield=260&frameworkcode=#subbfield
1c. Choose "marc21_field_260b.pl" from the dropdown next to "Plugin"
1d. Click "Save changes"
2a. "Add new record"
2b. Go to http://localhost:8081/cgi-bin/koha/cataloguing/addbiblio.pl?frameworkcode=
3. Click on tab "2" and scroll down to 260 "b"
4. Type in "Ori" into 260 subfield b
5. Some autocomplete suggestions should appear

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 7 Nick Clemens (kidclamp) 2023-11-09 20:28:54 UTC
Created attachment 158765 [details] [review]
Bug 35290: (follow-up) Perltidy formatting corrections

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Tomás Cohen Arazi (tcohen) 2023-11-24 19:45:57 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 9 Fridolin Somers 2023-11-24 20:56:40 UTC
--- a/cataloguing/ysearch.pl
+++ b/cataloguing/ysearch.pl
@@ -1,4 +1,4 @@
-#!/usr/bin/perl
+    #!/usr/bin/perl

Whooo is this normal ?
Comment 10 David Cook 2023-11-26 22:18:59 UTC
(In reply to Fridolin Somers from comment #9)
> --- a/cataloguing/ysearch.pl
> +++ b/cataloguing/ysearch.pl
> @@ -1,4 +1,4 @@
> -#!/usr/bin/perl
> +    #!/usr/bin/perl
> 
> Whooo is this normal ?

Looks like a bad perltidy to me.

It probably won't have any negative consequences overall when using Plack instead of CGI. But good to fix that I'd say.
Comment 11 Jonathan Druart 2023-12-01 08:16:27 UTC
Pushed to 21.11.x for 21.11.26
Comment 12 Marcel de Rooy 2024-01-18 15:01:04 UTC
(In reply to Fridolin Somers from comment #9)
> --- a/cataloguing/ysearch.pl
> +++ b/cataloguing/ysearch.pl
> @@ -1,4 +1,4 @@
> -#!/usr/bin/perl
> +    #!/usr/bin/perl
> 
> Whooo is this normal ?

We should catch that in qa tools ?
Comment 13 Marcel de Rooy 2024-01-18 15:06:32 UTC
(In reply to David Cook from comment #10)
> It probably won't have any negative consequences overall when using Plack
> instead of CGI. But good to fix that I'd say.

Done on 35835
Comment 14 David Cook 2024-09-05 03:18:32 UTC
Do we move this back to Koha project now?
Comment 15 Jonathan Druart 2024-09-05 07:02:19 UTC
(In reply to David Cook from comment #14)
> Do we move this back to Koha project now?

Yes, done.