Attachment 51826 Details for Bug 16593 – Bug 16593: Do not allow patrons to delete search history of others patrons

[patch] Bug 16593: Do not allow patrons to delete search history of others patrons

Bug-16593-Do-not-allow-patrons-to-delete-search-hi.patch (text/plain), 2.12 KB, created by Chris Cormack on 2016-05-26 11:20:05 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Cormack

Created: 2016-05-26 11:20:05 UTC

Size: 2.12 KB

patch

obsolete

>From ae584f1d5e95d24c738d1d86034ca456ce260269 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Thu, 26 May 2016 11:52:19 +0100
>Subject: [PATCH] Bug 16593: Do not allow patrons to delete search history of
> others patrons
>
>A malicious user can delete the search history of all other users by
>correctly guessing the ID value assigned to the victim's search. As
>searches are assigned values sequentially, an attacker could quickly
>remove the searches belonging to all of the application's users.
>
>To reproduce:
>Login with patron A
>launch a search
>Note the id generated for this search history:
>select id from search_history order by id desc limit 1;
>Login with patron B
>Hit /cgi-bin/koha/opac-search-history.pl?action=delete&id=<ID>
>Note that the row is deleted in the DB
>
>Test plan
>Confirm that this patch fixes the issue.
>The same test can be made at the staff interface
>
>Reported by Alex Middleton at Dionach
>
>Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
>---
> catalogue/search-history.pl | 3 ++-
> opac/opac-search-history.pl | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/catalogue/search-history.pl b/catalogue/search-history.pl
>index 4a8251f..f6714e1 100755
>--- a/catalogue/search-history.pl
>+++ b/catalogue/search-history.pl
>@@ -46,7 +46,8 @@ if ( $action eq 'delete' ) {
>         : q{};
>     C4::Search::History::delete(
>         {
>-            id => [ $cgi->param('id') ],
>+            userid => $loggedinuser,
>+            id     => [ $cgi->param('id') ],
>         }
>     );
>     # Redirecting to this same url so the user won't see the search history link in the header
>diff --git a/opac/opac-search-history.pl b/opac/opac-search-history.pl
>index 4858b5a..34b0e84 100755
>--- a/opac/opac-search-history.pl
>+++ b/opac/opac-search-history.pl
>@@ -103,7 +103,8 @@ unless ( $loggedinuser ) {
>         if ( @id ) {
>             C4::Search::History::delete(
>                 {
>-                    id => [ $cgi->param('id') ],
>+                    userid => $loggedinuser,
>+                    id     => [ $cgi->param('id') ],
>                 }
>             );
>         } else {
>-- 
>2.8.1

Actions: View | Diff | Splinter Review

Attachments on bug 16593: 51823 | 51826 | 52494