Bug 27715 - Possibly SQL injection in virtualshelves
Summary: Possibly SQL injection in virtualshelves
Status: Pushed to oldoldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Lists (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-02-17 11:22 UTC by Martin Renvoize
Modified: 2021-02-24 16:25 UTC (History)
13 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:
21.05.00,20.11.03,20.05.09,19.11.15


Attachments
Bug 27715: Remove unused build_dt_query (2.84 KB, patch)
2021-02-18 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27715: Remove unused dt_build_having (4.34 KB, patch)
2021-02-18 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27715: Add tests (2.50 KB, patch)
2021-02-18 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27715: Sanitize order by DT params (1.11 KB, patch)
2021-02-18 14:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27715: Add a deprecation notice (729 bytes, patch)
2021-02-18 14:55 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27715: Remove unused build_dt_query (2.90 KB, patch)
2021-02-19 08:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Remove unused dt_build_having (4.40 KB, patch)
2021-02-19 08:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Add tests (2.56 KB, patch)
2021-02-19 08:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Sanitize order by DT params (1.17 KB, patch)
2021-02-19 08:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Add a deprecation notice (794 bytes, patch)
2021-02-19 08:33 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Remove unused build_dt_query (2.90 KB, patch)
2021-02-19 09:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Remove unused dt_build_having (4.35 KB, patch)
2021-02-19 09:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Add tests (2.56 KB, patch)
2021-02-19 09:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Sanitize order by DT params (1.17 KB, patch)
2021-02-19 09:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Add a deprecation notice (794 bytes, patch)
2021-02-19 09:11 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Remove unused build_dt_query (3.06 KB, patch)
2021-02-19 12:26 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27715: Remove unused dt_build_having (4.51 KB, patch)
2021-02-19 12:26 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27715: Add tests (2.72 KB, patch)
2021-02-19 12:26 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27715: Sanitize order by DT params (1.32 KB, patch)
2021-02-19 12:26 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27715: Add a deprecation notice (945 bytes, patch)
2021-02-19 12:27 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27715: Use $dbh->quote_identifier to quote untrusted input (3.20 KB, patch)
2021-02-19 12:27 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27715: (follow-up) Remove accidentally introduced debugging statement (846 bytes, patch)
2021-02-19 12:27 UTC, Joonas Kylmälä
Details | Diff | Splinter Review
Bug 27715: Remove unused build_dt_query (3.05 KB, patch)
2021-02-19 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Remove unused dt_build_having (4.50 KB, patch)
2021-02-19 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Add tests (2.71 KB, patch)
2021-02-19 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Sanitize order by DT params (1.32 KB, patch)
2021-02-19 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Add a deprecation notice (945 bytes, patch)
2021-02-19 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Use $dbh->quote_identifier to quote untrusted input (3.26 KB, patch)
2021-02-19 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: (follow-up) Remove accidentally introduced debugging statement (911 bytes, patch)
2021-02-19 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 27715: Remove unused build_dt_query (3.15 KB, patch)
2021-02-20 09:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27715: Remove unused dt_build_having (4.11 KB, patch)
2021-02-20 09:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27715: Add tests (2.83 KB, patch)
2021-02-20 09:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27715: Sanitize order by DT params (1.42 KB, patch)
2021-02-20 09:58 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27715: Add a deprecation notice (1.02 KB, patch)
2021-02-20 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27715: Use $dbh->quote_identifier to quote untrusted input (3.36 KB, patch)
2021-02-20 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 27715: (QA follow-up) Remove conditional declaration (1.01 KB, patch)
2021-02-20 09:59 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2021-02-17 11:22:35 UTC
The "mDataProp_5" field in the "Lists" page below is vulnerable to time-based SQL injection.
An attacker can exploit this to read any information in the MariaDB database. This could
therefore be used to extract sensitive information such as usernames, password hashes and
library users' personally identifiable information.
https://bcp-staff.koha-ptfs.co.uk/cgi-bin/koha/virtualshelves/shelves.pl
The following proof of concept demonstrates that it is possible to inject arbitrary SQL
queries in the "mDataProp_5" field. The highlighted SQL command instructs the server to
pause the SQL query execution for 40 seconds.
POST https://bcp-staff.koha-ptfs.co.uk/cgi-bin/koha/svc/virtualshelves/search
[...]
mDataProp_4=dt_owner&bSortable_4=true&mDataProp_5=dt_sortby,(select*f
rom(select(sleep(40)))a)&bSortable_5=true&mDataProp_6=dt_created_on&b
Sortable_6=true&mDataProp_7=dt_modification_time&bSortable_7=true&mDa
taProp_8=dt_action&bSortable_8=false&iSortCol_0=5&sSortDir_0=asc&iSor
tingCols=1&type=1&shelfname=&owner=&sortby=&template_path=virtualshel
ves%2Ftables%2Fshelves_results.tt
As a result, the request above was executed in 41.269 milliseconds which indicates a
successful execution of the injected SQL query. This vulnerability can be leveraged to extract
information from the database. The following SQL statement selects the username of the
user running the database and the extracts the first character from the username.
Furthermore, the "IF()" function is used to compare if the extracted character matches the
test character provided. As an example, if the first character of the username is equal the
letter "k", then the database would execute "SLEEP(5)" statement, which would delay the
response by five seconds. Alternatively, if the first username's character was not "k", then
the response would only be delayed by one second. This example can be translated into the
following SQL query:
(select*from(SELECT+IF((SELECT+MID(user(),+1,+1)+AS+ExtractString)='k
',+SLEEP(5),+SLEEP(1)))b)

Confidential Project No: 500-1169Q1
Page 13 of 61 Date: 2021-02-11
Using this logic, it was possible to determine the rest of the characters and extract the full
username "koha_bcp@localhost". By automating this process, an authenticated attacker can
extract a wealth of information from the KOHA database. This includes personally
identifiable information of library users.
Impact: High
An attacker can execute arbitrary SQL queries and gain access to sensitive personally
identifiable information such as library user addresses, date of birth and contact details. This
information can be used for targeted social engineering attacks or identity theft.
Likelihood: Medium
SQL injection is a well-known vulnerability and some of the pages that are vulnerable are
publicly accessible and straightforward to exploit. A large number of automated tools exist
to exploit SQL injection. However, to exploit this issue an attacker would need to have an
administrative access to the "Lists" page.
Recommendation
Review all of the code used by the website and use prepared statements where possible for
all SQL calls. If this is not possible, ensure that all user input is satanized from dangerous
characters or use data type casting for all other data types.
Please see the following OWASP article for more information:
https://cheatsheetseries.owasp.org/cheatsheets/SQL_Injection_Prevention_Cheat_Sheet.ht
ml
Comment 1 Jonathan Druart 2021-02-18 14:52:12 UTC
Created attachment 116992 [details] [review]
Bug 27715: Remove unused build_dt_query
Comment 2 Jonathan Druart 2021-02-18 14:52:15 UTC
Created attachment 116993 [details] [review]
Bug 27715: Remove unused dt_build_having
Comment 3 Jonathan Druart 2021-02-18 14:52:18 UTC
Created attachment 116994 [details] [review]
Bug 27715: Add tests
Comment 4 Jonathan Druart 2021-02-18 14:52:21 UTC
Created attachment 116995 [details] [review]
Bug 27715: Sanitize order by DT params

We are not on the safe side when we build the ORDER BY clause from the
DataTables parameters.

I've started to limit the columns by using Koha::Objects->columns, but
for instance for the patron search we need (at least) the columns from
the branches, categories and members tables.
It seems easier, and still safe, to use a regex.
Comment 5 Jonathan Druart 2021-02-18 14:53:47 UTC
We will have to backport these patches into all the stable branches so please test carefully the list and patrons searches (filters, sort, etc.)
Comment 6 Jonathan Druart 2021-02-18 14:55:09 UTC
Created attachment 116998 [details] [review]
Bug 27715: Add a deprecation notice
Comment 7 Martin Renvoize 2021-02-19 08:33:25 UTC
Created attachment 117027 [details] [review]
Bug 27715: Remove unused build_dt_query

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2021-02-19 08:33:28 UTC
Created attachment 117028 [details] [review]
Bug 27715: Remove unused dt_build_having

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2021-02-19 08:33:31 UTC
Created attachment 117029 [details] [review]
Bug 27715: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2021-02-19 08:33:35 UTC
Created attachment 117030 [details] [review]
Bug 27715: Sanitize order by DT params

We are not on the safe side when we build the ORDER BY clause from the
DataTables parameters.

I've started to limit the columns by using Koha::Objects->columns, but
for instance for the patron search we need (at least) the columns from
the branches, categories and members tables.
It seems easier, and still safe, to use a regex.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Martin Renvoize 2021-02-19 08:33:38 UTC
Created attachment 117031 [details] [review]
Bug 27715: Add a deprecation notice

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2021-02-19 08:34:53 UTC
Thanks for jumping on this so quickly Jonathan :)

Great to see a bit of cleaning up going on too.. I hadn't dug far enough to spot that half these routines are no longer actively used!

Patches work as expected and close the loop nicely.

Signing off.
Comment 13 Martin Renvoize 2021-02-19 09:11:32 UTC
Created attachment 117032 [details] [review]
Bug 27715: Remove unused build_dt_query

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize 2021-02-19 09:11:36 UTC
Created attachment 117033 [details] [review]
Bug 27715: Remove unused dt_build_having

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2021-02-19 09:11:39 UTC
Created attachment 117034 [details] [review]
Bug 27715: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2021-02-19 09:11:43 UTC
Created attachment 117035 [details] [review]
Bug 27715: Sanitize order by DT params

We are not on the safe side when we build the ORDER BY clause from the
DataTables parameters.

I've started to limit the columns by using Koha::Objects->columns, but
for instance for the patron search we need (at least) the columns from
the branches, categories and members tables.
It seems easier, and still safe, to use a regex.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2021-02-19 09:11:46 UTC
Created attachment 117036 [details] [review]
Bug 27715: Add a deprecation notice

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Joonas Kylmälä 2021-02-19 10:29:46 UTC
(In reply to Martin Renvoize from comment #16)
> Created attachment 117035 [details] [review] [review]
> Bug 27715: Sanitize order by DT params
> 
> We are not on the safe side when we build the ORDER BY clause from the
> DataTables parameters.
> 
> I've started to limit the columns by using Koha::Objects->columns, but
> for instance for the patron search we need (at least) the columns from
> the branches, categories and members tables.
> It seems easier, and still safe, to use a regex.
> 
> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

I am not 100% convinced this protects us from all injections, there is so many unicode characters in \w. In C4/Utils/DataTables/Members.pm for this we use $dbh->quote_identifier($searchfield) – I think we might be able to use it here as well but have to do a bit more complex change. Please see https://metacpan.org/pod/DBI#quote_identifier for documentation.
Comment 19 Jonathan Druart 2021-02-19 11:54:52 UTC
I forgot about quote_identifier, and only tried quote (it didn't work).
Comment 20 Joonas Kylmälä 2021-02-19 11:57:06 UTC
I started now working on adding quote_identifier() sanitization to this. Will let you know the results.
Comment 22 Joonas Kylmälä 2021-02-19 12:26:47 UTC
Created attachment 117068 [details] [review]
Bug 27715: Remove unused build_dt_query

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 23 Joonas Kylmälä 2021-02-19 12:26:51 UTC
Created attachment 117069 [details] [review]
Bug 27715: Remove unused dt_build_having

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 24 Joonas Kylmälä 2021-02-19 12:26:56 UTC
Created attachment 117070 [details] [review]
Bug 27715: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 25 Joonas Kylmälä 2021-02-19 12:26:59 UTC
Created attachment 117071 [details] [review]
Bug 27715: Sanitize order by DT params

We are not on the safe side when we build the ORDER BY clause from the
DataTables parameters.

I've started to limit the columns by using Koha::Objects->columns, but
for instance for the patron search we need (at least) the columns from
the branches, categories and members tables.
It seems easier, and still safe, to use a regex.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 26 Joonas Kylmälä 2021-02-19 12:27:03 UTC
Created attachment 117072 [details] [review]
Bug 27715: Add a deprecation notice

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 27 Joonas Kylmälä 2021-02-19 12:27:07 UTC
Created attachment 117073 [details] [review]
Bug 27715: Use $dbh->quote_identifier to quote untrusted input

The sanitization using regex and \w class of characters might be
enough but given the vast number of unicode characters in \w and
possibility of in the future the database engines interpreting some of
those characters with special meaning it is better to wrap the column
identifier to quotes using $dbh->quote_identifier so it is only
interpreted as identifier and nothing else.
Comment 28 Joonas Kylmälä 2021-02-19 12:27:12 UTC
Created attachment 117074 [details] [review]
Bug 27715: (follow-up) Remove accidentally introduced debugging statement
Comment 29 Joonas Kylmälä 2021-02-19 12:29:26 UTC
I added the quotation part on top of the earlier fix, doesn't hurt to have two levels of defense? Albeit it is now a bit more complex...
Comment 30 Martin Renvoize 2021-02-19 12:52:25 UTC
Created attachment 117076 [details] [review]
Bug 27715: Remove unused build_dt_query

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 31 Martin Renvoize 2021-02-19 12:52:28 UTC
Created attachment 117077 [details] [review]
Bug 27715: Remove unused dt_build_having

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 32 Martin Renvoize 2021-02-19 12:52:32 UTC
Created attachment 117078 [details] [review]
Bug 27715: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 33 Martin Renvoize 2021-02-19 12:52:36 UTC
Created attachment 117079 [details] [review]
Bug 27715: Sanitize order by DT params

We are not on the safe side when we build the ORDER BY clause from the
DataTables parameters.

I've started to limit the columns by using Koha::Objects->columns, but
for instance for the patron search we need (at least) the columns from
the branches, categories and members tables.
It seems easier, and still safe, to use a regex.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 34 Martin Renvoize 2021-02-19 12:52:40 UTC
Created attachment 117080 [details] [review]
Bug 27715: Add a deprecation notice

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Comment 35 Martin Renvoize 2021-02-19 12:52:44 UTC
Created attachment 117081 [details] [review]
Bug 27715: Use $dbh->quote_identifier to quote untrusted input

The sanitization using regex and \w class of characters might be
enough but given the vast number of unicode characters in \w and
possibility of in the future the database engines interpreting some of
those characters with special meaning it is better to wrap the column
identifier to quotes using $dbh->quote_identifier so it is only
interpreted as identifier and nothing else.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Martin Renvoize 2021-02-19 12:52:47 UTC
Created attachment 117082 [details] [review]
Bug 27715: (follow-up) Remove accidentally introduced debugging statement

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize 2021-02-19 12:53:52 UTC
Nice teamwork there guys.. I've tested the followups and am really happy with them.  Signed off those two too
Comment 38 Marcel de Rooy 2021-02-20 09:58:48 UTC
Created attachment 117112 [details] [review]
Bug 27715: Remove unused build_dt_query

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 39 Marcel de Rooy 2021-02-20 09:58:52 UTC
Created attachment 117113 [details] [review]
Bug 27715: Remove unused dt_build_having

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Amended: removed warn.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 40 Marcel de Rooy 2021-02-20 09:58:56 UTC
Created attachment 117114 [details] [review]
Bug 27715: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Amended: chmod 755.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 41 Marcel de Rooy 2021-02-20 09:58:59 UTC
Created attachment 117115 [details] [review]
Bug 27715: Sanitize order by DT params

We are not on the safe side when we build the ORDER BY clause from the
DataTables parameters.

I've started to limit the columns by using Koha::Objects->columns, but
for instance for the patron search we need (at least) the columns from
the branches, categories and members tables.
It seems easier, and still safe, to use a regex.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 42 Marcel de Rooy 2021-02-20 09:59:03 UTC
Created attachment 117116 [details] [review]
Bug 27715: Add a deprecation notice

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 43 Marcel de Rooy 2021-02-20 09:59:07 UTC
Created attachment 117117 [details] [review]
Bug 27715: Use $dbh->quote_identifier to quote untrusted input

The sanitization using regex and \w class of characters might be
enough but given the vast number of unicode characters in \w and
possibility of in the future the database engines interpreting some of
those characters with special meaning it is better to wrap the column
identifier to quotes using $dbh->quote_identifier so it is only
interpreted as identifier and nothing else.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 44 Marcel de Rooy 2021-02-20 09:59:10 UTC
Created attachment 117118 [details] [review]
Bug 27715: (QA follow-up) Remove conditional declaration

Resolves:
FAIL critic # Variables::ProhibitConditionalDeclarations: Got 1 violation(s).

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 45 Marcel de Rooy 2021-02-20 10:14:19 UTC
koha-tmpl/intranet-tmpl/prog/en/modules/virtualshelves/shelves.tt:                    'sAjaxSource': "/cgi-bin/koha/svc/virtualshelves/search",

This svc needs catalogue permission.
The opac-shelves html does not run ajax svc script to get the info.
So the impact is less high.

Passed QA
Comment 46 Fridolin Somers 2021-02-23 13:34:42 UTC
Will be in this month release of stable branches
Comment 47 Victor Grousset/tuxayo 2021-02-23 18:08:21 UTC
Backported: Pushed to 19.11.x branch for 19.11.15
Comment 48 Andrew Fuerste-Henry 2021-02-23 18:27:03 UTC
Backported to 20.05.x for 20.05.09
Comment 49 Fridolin Somers 2021-02-24 16:25:47 UTC
Pushed to 20.11.x for 20.11.03