Bug 1953 - remove possible SQL injection attacks
Summary: remove possible SQL injection attacks
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Database (show other bugs)
Version: unspecified
Hardware: All All
: P3 normal (vote)
Assignee: Galen Charlton
QA Contact:
URL:
Keywords:
: 5849 (view as bug list)
Depends on:
Blocks:
 
Reported: 2008-03-18 01:16 UTC by Chris Cormack
Modified: 2012-10-25 23:09 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Follow up patch for typo (881 bytes, patch)
2011-03-10 14:23 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 Chris Cormack 2010-05-21 00:36:31 UTC


---- Reported by Andrew.moore@liblime.com 2008-03-18 13:16:05 ----

I've found a handful of SQL queries that don't use placeholders and bind variables, but instead have variables passed directly into them. These may allow SQL injection attacks. I plan on refactoring them so that they use placeholders instead.



---- Additional Comments From Andrew.moore@liblime.com 2008-03-18 13:17:04 ----

These are the modules and number of queries that I spotted on my first walk through the C4 directory:

Acquisition.pm          6
Branch.pm               1
Calendar.pm             11
Context.pm              1
Items.pm                3
Koha.pm                 2
Log.pm                  2
Members.pm              3
NewsChannels.pm         7
Overdues.pm             1
Serials.pm              3
Suggestions.pm          1




---- Additional Comments From Andrew.moore@liblime.com 2008-04-11 09:39:37 ----

I've submitted patches for all of the subs in C4::Acquisition that I could find. I think there were about 6 of them.

These two have not been applied quite yet.
http://lists.koha.org/pipermail/koha-patches/2008-April/000116.html
http://lists.koha.org/pipermail/koha-patches/2008-April/000123.html

I'll get to the other modules eventually, but probably not right away.
-Andy




---- Additional Comments From gmcharlt@gmail.com 2008-05-12 08:01:59 ----

Submitted patch for circ/ysearch.pl that among other things fixes this bug for that script.



---- Additional Comments From Andrew.moore@liblime.com 2008-05-12 12:37:49 ----

I submitted a patch to fix C4::Branch::GetBranches.

http://lists.koha.org/pipermail/koha-patches/2008-May/000401.html

There is also C4::Branch::get_branchinfos_of, but it uses C4::Koha::get_infos_of and can't pass bind bind parameters to it. So, it's a more invasive change. I'll leave it for later.







---- Additional Comments From Andrew.moore@liblime.com 2008-05-13 10:03:36 ----

Two patches for C4::Calendar:
http://lists.koha.org/pipermail/koha-patches/2008-May/000422.html
http://lists.koha.org/pipermail/koha-patches/2008-May/000421.html

I can't quite figure out how C4::Calendar::delete_holiday is working, but there are a few SQL statements in it to fix. Perhaps when http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=2106 is addressed, it will make it easier to fix. For now, I'm moving to another module.






---- Additional Comments From Andrew.moore@liblime.com 2008-07-23 12:28:42 ----

patches for C4::Context submitted
http://markmail.org/message/uovswyowd6jkqobr




---- Additional Comments From Andrew.moore@liblime.com 2008-07-25 10:06:10 ----

patch submitted for one query in C4::Items::GetItemsForInventory
http://markmail.org/message/f47iannryrno3tgn
I had indicated that there were two questionable queries in C4::Items, but I don't see the other, so I'm considering this package done.




---- Additional Comments From Andrew.moore@liblime.com 2008-07-25 13:51:00 ----

patch for C4::Koha::get_itemtypeinfos_of submitted:
http://markmail.org/message/sbrnb27ysojogteb
I see at least one more in C4::Koha.




---- Additional Comments From Andrew.moore@liblime.com 2008-07-29 09:47:54 ----

patches to refactor C4::Koha::displayServers submitted:
http://markmail.org/message/37yoiiasfgcz3jb4

I can't find any more in C4::Koha.

I'll come back to the rest later. I have a higher priority bug to work on.




---- Additional Comments From Andrew.moore@liblime.com 2008-07-30 11:49:15 ----

I appear to have written a test that doesn't always work right. I'll get to this soon:

# KohaTest::Items::GetItemsForInventory->basic_usage
ok 1076 - we have a call number to search for: 8eCsPId
ok 1077 - We were able to call GetItemsForInventory with our call number isa ARRAY
ok 1078 - ...and we found only one item
not ok 1079 - ...and the item we found has the right itemnumber

#   Failed test '...and the item we found has the right itemnumber'
#   at lib/KohaTest/Items/GetItemsForInventory.pm line 80.
#   (in KohaTest::Items::GetItemsForInventory->basic_usage)
#          got: '47'
#     expected: '43'
# $items = [
#            {
#              'itemnumber' => '47',
#              'datelastseen' => '07/30/2008',
#              'biblionumber' => '43',
#              'itemcallnumber' => '8eCsPId',
#              'author' => 'Twain, Mark',
#              'title' => 'The Adventures of Huckleberry Finn Test 1',
#              'barcode' => '123456782gaAXGS'
#            }
#          ];
# 
# KohaTest::Items::GetItemsForInventory->date_last_seen
ok 1080 - we have a call number to search for: 8eCsPId
ok 1081 - We were able to call GetItemsForInventory with our call number isa ARRAY
ok 1082 - ...and we found only one item
not ok 1083 - ...and the item we found has the right itemnumber

#   Failed test '...and the item we found has the right itemnumber'
#   at lib/KohaTest/Items/GetItemsForInventory.pm line 106.
#   (in KohaTest::Items::GetItemsForInventory->date_last_seen)
#          got: '47'
#     expected: '43'
ok 1084 - We were able to call GetItemsForInventory with our call number isa ARRAY
ok 1085 - ...and we found no items
# 
# KohaTest::Items::GetItemsForInventory->methods
ok 1086 - C4::Items->can(...)
# 
# KohaTest::Items::GetItemsForInventory->missing_parameters
not ok 1087 - missing parameters # TODO GetItemsForInventory should fail when missing required parameters

#   Failed (TODO) test 'missing parameters'
#   at lib/KohaTest/Items/GetItemsForInventory.pm line 63.
#   (in KohaTest::Items::GetItemsForInventory->missing_parameters)
# $items = [];
ok 1088 - truncated tables
ok 1089 - created bookseller: 1





---- Additional Comments From Andrew.moore@liblime.com 2008-07-31 08:01:14 ----

patch sent to address bug mentioned in Comment #10
http://markmail.org/message/tvxxmtdlzhblmxdg




---- Additional Comments From nengard@gmail.com 2009-12-23 17:58:55 ----

This bug hasn't been touched in nearly a year, please revisit and test and close if appropriate.



--- Bug imported by chris@bigballofwax.co.nz 2010-05-21 00:36 UTC  ---

This bug was previously known as _bug_ 1953 at http://bugs.koha.org/cgi-bin/bugzilla3/show_bug.cgi?id=1953

Actual time not defined. Setting to 0.0
The original reporter of this bug does not have
   an account here. Reassigning to the person who moved
   it here: chris@bigballofwax.co.nz.
   Previous reporter was Andrew.moore@liblime.com.
The original assignee of this bug does not have
   an account here. Reassigning to the default assignee
   for the component, gmcharlt@gmail.com.
   Previous assignee was Andrew.moore@liblime.com.

Comment 1 Chris Cormack 2011-02-26 07:04:04 UTC
Another patch for this pushed
Comment 2 Chris Cormack 2011-02-27 19:45:59 UTC
Pushed another patch
Comment 3 Marcel de Rooy 2011-02-28 08:32:45 UTC
Follow up fix sent for typo in security patch
Comment 4 MJ Ray (software.coop) 2011-03-05 10:34:05 UTC
This bug is mentioned in:
bug 1953: Reduce risk of	possible tainted supplierid call to C4::Serials::GetLateIssues http://lists.koha-community.org/pipermail/koha-patches/2011-February/013933.html
Security Bugfix:	Bug 1953 Adding Placeholders to SQL To Avoid Potential	Injection Attacks http://lists.koha-community.org/pipermail/koha-patches/2011-February/013935.html
bug 1953: Reduce	risk of possible tainted supplierid call to	C4::Serials::GetLateIssues http://lists.koha-community.org/pipermail/koha-patches/2011-February/013936.html
Comment 5 Marcel de Rooy 2011-03-10 14:21:46 UTC
*** Bug 5849 has been marked as a duplicate of this bug. ***
Comment 6 Marcel de Rooy 2011-03-10 14:23:48 UTC Comment hidden (obsolete)
Comment 7 Katrin Fischer 2012-04-02 15:58:49 UTC
All patches of this bug have been pushed. Closing this bug.