Bug 10033 - dangerous query in _koha_modify_item
Summary: dangerous query in _koha_modify_item
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-12 15:44 UTC by Fridolin Somers
Modified: 2013-12-05 20:09 UTC (History)
2 users (show)

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


Attachments
Proposed patch (1.14 KB, patch)
2013-04-12 15:57 UTC, Fridolin Somers
Details | Diff | Splinter Review
Proposed patch (1.14 KB, patch)
2013-04-12 16:00 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 10033 - dangerous query in _koha_modify_item (1.21 KB, patch)
2013-04-19 12:34 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 10033 - dangerous query in _koha_modify_item (1.31 KB, patch)
2013-04-29 14:56 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 Fridolin Somers 2013-04-12 15:44:27 UTC
The SQL query build in C4::Items::_koha_modify_item performs an update on a row of items table identified by itemnumber.
Actually the query is build using a hash of datas : 
    for my $key ( keys %$item ) {
        $query.="$key=?,";
        push @bind, $item->{$key};
    }
But this hash contains 'itemnumber' key, so you get an update including the primary key.
It is actually harmless but may be dangerous.
Comment 1 Fridolin Somers 2013-04-12 15:57:56 UTC Comment hidden (obsolete)
Comment 2 Fridolin Somers 2013-04-12 16:00:49 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2013-04-19 12:34:49 UTC Comment hidden (obsolete)
Comment 4 Marcel de Rooy 2013-04-29 14:56:45 UTC
Created attachment 17794 [details] [review]
Bug 10033 - dangerous query in _koha_modify_item

The SQL query build in C4::Items::_koha_modify_item performs an update on a row of items table identified by itemnumber.
Actually the query is build using a hash of datas :
    for my $key ( keys %$item ) {
        $query.="$key=?,";
        push @bind, $item->{$key};
    }
But this hash contains 'itemnumber' key, so you get an update including the primary key.
It is actually harmless but may be dangerous.

This patch simply skips itemnumber key in above loop.

Test plan :
Check you can create and modify items.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2013-04-29 14:59:43 UTC
QA Comment:
Very small patch that probably does prevent a fictive situation to occur. But does not harm either. Tested adding and editing item.

Out of scope, but did you notice that the error message from this same routine is just ignored in the calling routine?

Passed QA
Comment 6 Jared Camins-Esakov 2013-05-01 12:17:23 UTC
This patch has been pushed to master and 3.12.x.
Comment 7 Chris Cormack 2013-05-09 07:25:22 UTC
Pushed to 3.10.x and 3.8.x will be in 3.10.6 and 3.8.13