Attachment 163194 Details for Bug 19613 – Bug 19613: Scrub borrowers fields: borrowernotes opacnote

[patch] Bug 19613: Scrub borrowers fields: borrowernotes opacnote

Bug-19613-Scrub-borrowers-fields-borrowernotes-opa.patch (text/plain), 2.90 KB, created by Jonathan Druart on 2024-03-15 10:48:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2024-03-15 10:48:07 UTC

Size: 2.90 KB

patch

obsolete

>From 1d5d524c20d483190db6e5bf09a0df23cdb723cd Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Fri, 15 Mar 2024 11:37:43 +0100
>Subject: [PATCH] Bug 19613: Scrub borrowers fields: borrowernotes opacnote
>
>To prevent XSS
>---
> Koha/Patron.pm               |  7 +++++++
> t/db_dependent/Koha/Patron.t | 34 +++++++++++++++++++++++++++++++++-
> 2 files changed, 40 insertions(+), 1 deletion(-)
>
>diff --git a/Koha/Patron.pm b/Koha/Patron.pm
>index 5b0bd9ea7f5..97e5380fb7c 100644
>--- a/Koha/Patron.pm
>+++ b/Koha/Patron.pm
>@@ -30,6 +30,7 @@ use C4::Auth qw( checkpw_hash );
> use C4::Context;
> use C4::Letters qw( GetPreparedLetter EnqueueLetter SendQueuedMessages );
> use C4::Log qw( logaction );
>+use C4::Scrubber;
> use Koha::Account;
> use Koha::ArticleRequests;
> use Koha::AuthUtils;
>@@ -231,6 +232,12 @@ sub store {
>                 if defined $self->relationship
>                 and $self->relationship eq "";
> 
>+            for my $note_field ( qw( borrowernotes opacnote ) ) {
>+                if ( !$self->in_storage || $self->_result->is_column_changed($note_field) ) {
>+                    $self->$note_field(C4::Scrubber->new('comment')->scrub($self->$note_field));
>+                }
>+            }
>+
>             unless ( $self->in_storage ) {    #AddMember
> 
>                 # Generate a valid userid/login if needed
>diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t
>index e07e90eaf82..a953c3b93a2 100755
>--- a/t/db_dependent/Koha/Patron.t
>+++ b/t/db_dependent/Koha/Patron.t
>@@ -19,7 +19,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 32;
>+use Test::More tests => 33;
> use Test::Exception;
> use Test::Warn;
> use Time::Fake;
>@@ -2234,3 +2234,35 @@ subtest 'guarantor requirements tests' => sub {
>     'Koha::Exceptions::Patron::Relationship::NoGuarantor',
>         'Exception thrown when guarantor is deleted.';
> };
>+
>+subtest 'Scrub the note fields' => sub {
>+    plan tests => 4;
>+
>+    $schema->storage->txn_begin;
>+
>+    my $bad_message     = '<script>alert("booh!")</script><span>all</span><b>good</b>now';
>+    my $cleaned_message = '<span>all</span><b>good</b>now';
>+    my $tmp_patron      = $builder->build_object( { class => 'Koha::Patrons' } );
>+    my $patron_data     = $tmp_patron->unblessed;
>+    $tmp_patron->delete;
>+    delete $tmp_patron->{borrowernumber};
>+
>+    my $patron = Koha::Patron->new(
>+
>+        {
>+            %$patron_data,
>+            borrowernotes => $bad_message, opacnote => $bad_message,
>+        }
>+    )->store;
>+
>+    is( $patron->get_from_storage->borrowernotes, $cleaned_message );
>+    is( $patron->get_from_storage->opacnote,      $cleaned_message );
>+
>+    $patron->borrowernotes($bad_message)->store;
>+    $patron->opacnote($bad_message)->store;
>+
>+    is( $patron->get_from_storage->borrowernotes, $cleaned_message );
>+    is( $patron->get_from_storage->opacnote,      $cleaned_message );
>+
>+    $schema->storage->txn_rollback;
>+};
>-- 
>2.34.1

Actions: View | Diff | Splinter Review

Attachments on bug 19613: 69104 | 69109 | 163194 | 163498 | 163585 | 163586 | 164211 | 164212