Bug 10033

Summary: dangerous query in _koha_modify_item
Product: Koha Reporter: Fridolin Somers <fridolin.somers>
Component: CatalogingAssignee: Fridolin Somers <fridolin.somers>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: minor    
Priority: P5 - low CC: chris, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Proposed patch
Proposed patch
Bug 10033 - dangerous query in _koha_modify_item
Bug 10033 - dangerous query in _koha_modify_item

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