Bugzilla – Attachment 171157 Details for
Bug 37786
members/cancel-charge.pl needs CSRF protection
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 37786: members/cancel-charge.pl needs CSRF protection
Bug-37786-memberscancel-chargepl-needs-CSRF-protec.patch (text/plain), 4.49 KB, created by
Phil Ringnalda
on 2024-09-06 22:30:45 UTC
(
hide
)
Description:
Bug 37786: members/cancel-charge.pl needs CSRF protection
Filename:
MIME Type:
Creator:
Phil Ringnalda
Created:
2024-09-06 22:30:45 UTC
Size:
4.49 KB
patch
obsolete
>From 57247cc7ed70f47f22ed4440a04016cb083880f7 Mon Sep 17 00:00:00 2001 >From: Phil Ringnalda <phil@chetcolibrary.org> >Date: Fri, 6 Sep 2024 14:33:17 -0700 >Subject: [PATCH] Bug 37786: members/cancel-charge.pl needs CSRF protection > >members/cancel-charge.pl will take either a POST or a GET, and as long as the >accountline_id it is passed can be cancelled, will cancel it. That means any >link you click anywhere while logged in to Koha might cancel a charge. It also >takes a borrowernumber which isn't used for the cancelling, only to determine >what account to show after a charge is cancelled, letting a malicious link >show an account other than the one whose charge was just cancelled. > >Test plan: > 1. Without the patch, Circulation - Checkout - search for the 'koha' patron > you log in as > 2. Accounting - Create manual invoice - Make it a Manual fee of 100.00 and > Save > 3. Pretending it's a well-disguised link in a spear-phishing email, load > http://localhost:8081/cgi-bin/koha/members/cancel-charge.pl?borrowernumber=5&accountlines_id=1 > 4. You are now looking at charges for the patron Acosta, Edna rather than for > the patron koha, but if you look at the patron koha, its 100.00 charge > has been cancelled. > 5. Apply patch and reset_all (or if you don't, you'll have to manually adjust > the link to reflect the charge being accountlines_id 3 rather than 1) > 6. Circulation - Checkout - search for the 'koha' patron you log in as > 7. Accounting - Create manual invoice - Make it a Manual fee of 100.00 and > Save > 8. Click the link http://localhost:8081/cgi-bin/koha/members/cancel-charge.pl?borrowernumber=5&accountlines_id=1 > 9. You got a 403 because you didn't pass the op cud-cancel, but if you did > pass that op, you would also get a 403 for having a cud- op in a GET (and > if you POST, you won't have a csrf_token) >10. Checkout - search for koha - Accounting - Cancel charge >11. Having done it the right way, you're now on koha's list of transactions, > where you can see you just cancelled it > >Sponsored-by: Chetco Community Public Library >--- > .../prog/en/modules/members/boraccount.tt | 2 +- > members/cancel-charge.pl | 30 +++++++++++-------- > 2 files changed, 19 insertions(+), 13 deletions(-) > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >index 38a9edf174..bf85b86e92 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/boraccount.tt >@@ -190,7 +190,7 @@ > [% IF account.is_debit && account.amount == account.amountoutstanding && account.status != 'CANCELLED' && !(account.debit_type_code == 'PAYOUT') %] > <form method="post" action="/cgi-bin/koha/members/cancel-charge.pl"> > [% INCLUDE 'csrf-token.inc' %] >- <input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]"> >+ <input type="hidden" name="op" value="cud-cancel"> > <input type="hidden" name="accountlines_id" value="[% account.accountlines_id | html %]"> > <button type="submit" class="btn btn-default btn-xs cancel-action"> > <i class="fa fa-ban"></i> >diff --git a/members/cancel-charge.pl b/members/cancel-charge.pl >index 5748fa54c4..0b6d756cd0 100755 >--- a/members/cancel-charge.pl >+++ b/members/cancel-charge.pl >@@ -32,15 +32,21 @@ my $flags = { > my $type = 'intranet'; > my ($user, $cookie) = C4::Auth::checkauth($cgi, $authnotrequired, $flags, $type); > >-my $borrowernumber = $cgi->param('borrowernumber'); >-my $accountlines_id = $cgi->param('accountlines_id'); >- >-my $charge = Koha::Account::Lines->find($accountlines_id); >-$charge->cancel( >- { >- branch => C4::Context->userenv->{'branch'}, >- staff_id => C4::Context->userenv->{'number'} >- } >-); >- >-print $cgi->redirect('/cgi-bin/koha/members/boraccount.pl?borrowernumber=' . $borrowernumber); >+my $op = $cgi->param('op') // q{}; >+ >+if ( $op eq "cud-cancel" ) { >+ my $accountlines_id = $cgi->param('accountlines_id'); >+ >+ my $charge = Koha::Account::Lines->find($accountlines_id); >+ my $borrowernumber = $charge->patron->borrowernumber; >+ $charge->cancel( >+ { >+ branch => C4::Context->userenv->{'branch'}, >+ staff_id => C4::Context->userenv->{'number'} >+ } >+ ); >+ print $cgi->redirect( '/cgi-bin/koha/members/boraccount.pl?borrowernumber=' . $borrowernumber ); >+ exit; >+} >+ >+print $cgi->redirect('/cgi-bin/koha/errors/403.pl'); >-- >2.44.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 37786
:
171157
|
171543