Bug 14521 - SQL injection in local use system preferences
Summary: SQL injection in local use system preferences
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-07-13 04:03 UTC by David Cook
Modified: 2016-12-05 21:23 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14521 - SQL injection in local use system preferences (1.72 KB, patch)
2015-07-13 04:11 UTC, David Cook
Details | Diff | Splinter Review
Bug 14521 - SQL injection in local use system preferences (1.77 KB, patch)
2015-07-13 04:16 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 14521 - SQL injection in local use system preferences (1.85 KB, patch)
2015-07-13 08:27 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2015-07-13 04:03:36 UTC
If you look at "sub StringSearch" in systempreferences.pl, you'll see that the WHERE clause of the select statement uses the results of "get_local_prefs()" without sanitizing the input or using bind parameters.

This means you can define your own local use system preference of "') or '1' = '1' -- " and you'll see all the system preferences (or at least the first 100 due to some pagination code).

The DBI docs say "Multiple SQL statements may not be combined in a single statement handle ($sth), although some databases and drivers do support this (notably Sybase and SQL Server)." (http://search.cpan.org/~timb/DBI-1.633/DBI.pm), so you may not be able to add additional "update" or "drop" statements.

However, Chris Cormack pointed me to http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14412 where it's clear you could do things like time-based attacks exploiting this SQL injection vulnerability.

Of course, if you're in the staff client and meddling with system preferences, you're probably already able to see most data, but the fact that you're able to exploit this vulnerability is in itself cause for concern...
Comment 1 Chris Cormack 2015-07-13 04:10:32 UTC
(In reply to David Cook from comment #0)

> 
> Of course, if you're in the staff client and meddling with system
> preferences, you're probably already able to see most data, but the fact
> that you're able to exploit this vulnerability is in itself cause for
> concern...

The trick is, you don't have to be in the staff client, the danger is in tricking someone into clicking on a link you have crafted. Usually combined with some XSS or something, so they think they are modifying a suggestion, but its doing nasty things to the db instead.
Comment 2 David Cook 2015-07-13 04:11:45 UTC Comment hidden (obsolete)
Comment 3 Chris Cormack 2015-07-13 04:16:58 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-07-13 08:27:52 UTC
Created attachment 40964 [details] [review]
Bug 14521 - SQL injection in local use system preferences

This patch fixes a SQL injection vulnerability in the local use
system preferences.

_TEST PLAN_

Before applying:

1) Go to Global System Preferences
2) Click on the "Local use" tab
3) Add a new preference with the value "') or '1' = '1' -- "
(be sure to include the space at the end after the comment --).
4) When the page refreshes, you should now see about 99 other system
preferences which shouldn't be showing up.

5) Apply the patch

6) Refresh the page
7) Note that you now only see a system preference for "') or '1' = '1' -- "
and the other actual local use system preferences.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 5 Tomás Cohen Arazi 2015-07-20 13:28:13 UTC
Patch pushed to master.

Thanks David!
Comment 6 Chris Cormack 2015-07-21 04:02:28 UTC
Pushed to 3.20.x will be in 3.20.2
Comment 7 Mason James 2015-07-25 13:13:57 UTC
Pushed to 3.16.x, will be in 3.16.13
Comment 8 Fridolin SOMERS 2015-08-21 12:02:03 UTC
Looks like it is pushed in 3.18.10