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.
Created attachment 17401 [details] [review] Proposed patch See commit comment. 1 line patch
Created attachment 17402 [details] [review] Proposed patch
Created attachment 17541 [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>
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>
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
This patch has been pushed to master and 3.12.x.
Pushed to 3.10.x and 3.8.x will be in 3.10.6 and 3.8.13