Bug 22543 - Patron might be logged in again using browser back button
Summary: Patron might be logged in again using browser back button
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Magnus Enger
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 24145 26191
  Show dependency treegraph
 
Reported: 2019-03-20 03:03 UTC by Katrin Fischer
Modified: 2020-08-11 11:21 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00


Attachments
Bug 22543 - Prevent "back and refresh attack" (2.40 KB, patch)
2019-09-06 08:09 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 22543 - Prevent "back and refresh attack" (2.76 KB, patch)
2019-09-10 07:31 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 22543: Prevent "back and refresh attack" (2.81 KB, patch)
2019-09-10 16:10 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 22543: Prevent "back and refresh attack" (2.90 KB, patch)
2019-09-12 13:32 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2019-03-20 03:03:09 UTC
I've got a report that it might be possible to re-login a user using the back button in the browser. After some testing, this appears to be a way to do it:

1) Log into a Koh ausing URL parameters:
?userid=...&password=...&koha_login_context=opac
2) Log out
3) Use browser back buttons

I didn't manage in Firefox and Chromium, but in IE 11 so far with 18.11.
Comment 1 Owen Leonard 2019-03-21 18:40:34 UTC
I was able to reproduce this problem in master using IE 11 and URL parameters.

The behavior in IE 11 using the login form is the same as in Firefox: Backing up after logging out shows you the "Document expired" message. Reloading that page (to resubmit the login form) does log you in again. I think that's standard browser behavior.

I could not reproduce the problem in 18.05.10 using Edge and URL parameters. A security bug in IE 11?
Comment 2 Katrin Fischer 2019-03-21 18:48:26 UTC
There is some code in checkauth that looks like it was meant to prevent this. I wonder if it doesn't work across all browsers and browser versions?

# In case, that this request was a login attempt, we want to prevent that users can repost the opac login
# request. We therefore redirect the user to the requested page again without the login parameters.
# See Post/Redirect/Get (PRG) design pattern: https://en.wikipedia.org/wiki/Post/Redirect/Get
if ( $type eq "opac" && $query->param('password') && $query->param('userid') ) {
    print $query->redirect(-uri => $query->url(-relative=>1), -cookie => $cookie, -status=>'303 See other');

The fix suggested by the one reporting this bug to me was:

if ( $type eq "opac" && $query->param('koha_login_context') && $query->param('password') && $query->param('userid') ) {
    my $uri = URI->new($query->url(-relative=>1, -query_string=>1));
    $uri->query_param_delete('userid');
    $uri->query_param_delete('password');
    $uri->query_param_delete('koha_login_context');
    print $query->redirect(-uri => $uri->as_string, -cookie => $cookie, -status=>'303 See other');
    exit;
}
Comment 3 Katrin Fischer 2019-03-21 19:00:31 UTC
(In reply to Katrin Fischer from comment #2)
> There is some code in checkauth that looks like it was meant to prevent
> this. I wonder if it doesn't work across all browsers and browser versions?
> 
> # In case, that this request was a login attempt, we want to prevent that
> users can repost the opac login
> # request. We therefore redirect the user to the requested page again
> without the login parameters.
> # See Post/Redirect/Get (PRG) design pattern:
> https://en.wikipedia.org/wiki/Post/Redirect/Get
> if ( $type eq "opac" && $query->param('password') && $query->param('userid')
> ) {
>     print $query->redirect(-uri => $query->url(-relative=>1), -cookie =>
> $cookie, -status=>'303 See other');
> 
> The fix suggested by the one reporting this bug to me was:
> 
> if ( $type eq "opac" && $query->param('koha_login_context') &&
> $query->param('password') && $query->param('userid') ) {
>     my $uri = URI->new($query->url(-relative=>1, -query_string=>1));
>     $uri->query_param_delete('userid');
>     $uri->query_param_delete('password');
>     $uri->query_param_delete('koha_login_context');
>     print $query->redirect(-uri => $uri->as_string, -cookie => $cookie,
> -status=>'303 See other');
>     exit;
> }

I just realized (or better was told by Joubu) that the first bit is not in master. It was an earlier attempt to fix the issue.
Comment 4 Magnus Enger 2019-08-22 12:05:13 UTC
I can reproduce this in Chromium, without fiddling with any URL parameters: 

1. Go to the front page of the OPAC
2. Log in and look at the "your summary" page
3. Log out
4. Click the "Back" button once or twice and get a page that says something like "Confirm new submission of form" and ERR_CACHE_MISS. 
5. Reload the page and get a dialog that says something like "Confirm new submission of form"
6. Click on "Continue" 

And voila, you are now logged in as the first patron! 

As Owen says, this is probably standard browser behaviour. The browser has no concept of login forms, where the content should not be reposted on a page refersh. So we probably have to be clever to stop this from happening. 

The only solution I can think of is something similar to how we handle CSRF: Issue some token with the login form that can only be submitted once and check that as part of the login process. 

I should also mention that this issue was identified and reported by a customer/library (running 18.11.05), which has now shut down their "kiosk" computers, because they consider this too big a security problem.
Comment 5 Magnus Enger 2019-08-22 12:12:44 UTC
In Firefox 68.0.2:

- Start on the Front page of the OPAC
- Log in
- Log out
- Click on Back
- Reload the page nd get:

Document Expired
This document is no longer available.
The requested document is not available in Firefox’s cache.
As a security precaution, Firefox does not automatically re-request sensitive documents.
Click Try Again to re-request the document from the website.

- Click on "Try again" and get:

To display this page, Firefox must send information that will repeat any action (such as a search or order confirmation) that was performed earlier.
[Cancel] [Resend]

- Click on "Resend" and you are logged in
Comment 6 Magnus Enger 2019-08-22 12:41:08 UTC
Some links that might be relevant:

https://en.wikipedia.org/wiki/Post/Redirect/Get

https://www.owasp.org/index.php/OWASP_Application_Security_FAQ#Is_it_really_required_to_redirect_the_user_to_a_new_page_after_login.3F

https://resources.infosecinstitute.com/browser-based-vulnerabilities-in-web-applications/ Section on "Back and refresh attack"

Mahara had this problem, but fixed it "Browser back and refresh button attack vulnerability": 
https://bugs.launchpad.net/mahara/+bug/1770561

From these it looks like the solution is to do the login in one script, e.g. opac-login.pl and then redirect to something like opac-user.pl to display the content. This works because the login form is submitted to opac-login.pl, but the user never sees this page, and can thus not reload it, because she is redirected to opac-user.pl behind the scenes.
Comment 7 Magnus Enger 2019-08-22 13:12:19 UTC
It looks like LMSCloud has proposed a solution for this here:
https://github.com/LMSCloud/Koha-LMSCloud/commit/5da5a0fb9259b371e15ef9b5b66253979e364859
Comment 8 Roger Grossmann 2019-08-30 06:26:52 UTC
The fix mentioned by Magnus was corrected later with the following commit:

https://github.com/LMSCloud/Koha-LMSCloud/commit/74a7fe0f0c5b2ce0d65bd26452c6dcaf0a7f65ad

With this last fix the problem was solved. It performs the a opac login action if login parameter (koha_login_context, userid, password) are provided and performs a redirect without the three parameters. Other parameters remain on the redirect URL.

A RE-POST action will be prevented because the original POST request in the Browser history will be replaced with the redirect URL (Post/Redirect/Get-Pattern).
Comment 9 Jonathan Druart 2019-09-01 18:19:55 UTC
(In reply to Roger Grossmann from comment #8)
> The fix mentioned by Magnus was corrected later with the following commit:

Could you submit a patch here?
Comment 10 Magnus Enger 2019-09-02 06:34:26 UTC
(In reply to Jonathan Druart from comment #9)
> (In reply to Roger Grossmann from comment #8)
> > The fix mentioned by Magnus was corrected later with the following commit:
> 
> Could you submit a patch here?

A patch from someone who already fixed this would be super awesome!
Comment 11 Magnus Enger 2019-09-06 08:09:56 UTC
Created attachment 92615 [details] [review]
Bug 22543 - Prevent "back and refresh attack"

To reproduce and test:
- Log into the OPAC, you are taken to /cgi-bin/koha/opac-user.pl
- Log out, you are taken to /cgi-bin/koha/opac-main.pl?logout.x=1
- Click "Back", you are taken to /cgi-bin/koha/opac-user.pl
- Reload the page, you see an error like "Confirm new submission
  of form"
- Reload the page again and confirm the submission of the form
- You are now logged in to the OPAC again!
- Log out again
- Apply this patch
- Log in to the OPAC, you are taken to /cgi-bin/koha/opac-user.pl
- Log out, you are taken to /cgi-bin/koha/opac-main.pl?logout.x=1
- Click back, you are taken to /cgi-bin/koha/opac-user.pl
- No matter how many times you reload /cgi-bin/koha/opac-user.pl,
  you should not see anything other than the login form.

The messages and errors pages you see related to resubmitting the
form might differ from the ones described here, depending on what
browser you use. I tested in Chromium 76.0.x.

This fix was originally proposed by LMSCloud:
https://github.com/LMSCloud/Koha-LMSCloud/commit/74a7fe0f0c5b2ce0d65bd26452c6dcaf0a7f65ad
Comment 12 Magnus Enger 2019-09-06 08:13:33 UTC
Sorry, I got impatient. :-) If LMSCloud wants to submit this as their own patch I will be happy to sign off on it. If they want more/some other form of credit in the commit message I will be happy to revise it. 

This patch: 
https://github.com/LMSCloud/Koha-LMSCloud/commit/74a7fe0f0c5b2ce0d65bd26452c6dcaf0a7f65ad
also use'es a couple of modules at the top of C4::Auth: 

use URI;
use URI::QueryParam;

I was able to make the fix work without these, so I left them out of the patch I submitted.

I have not tested this with SSO solutions like CAS, LDAP or Shibboleth.
Comment 13 Roger Grossmann 2019-09-06 08:48:36 UTC
Sorry for the delay Magnus. I was traveling not finding the time ...
I am absolutely fine/happy if you submit the patch and move forward with the patch.
Good to know that it works without the extra includes.
Thank you!
Comment 14 Magnus Enger 2019-09-09 13:11:51 UTC
Sorry, but the proposed solution breaks SCO. If you apply it, then go to /cgi-bin/koha/sco/sco-main.pl (with all the necessary sysprefs for SCO set) you will be redirected a number of times to /cgi-bin/koha/sco/sco-main.pl?sco_user_login=1 and after 10 attempts get ERR_TOO_MANY_REDIRECTS in the browser (at least Chromium).
Comment 15 Roger Grossmann 2019-09-09 14:25:18 UTC
If you change line:

        if ( $type eq "opac" && $query->param('koha_login_context') && $query->param('password') && $query->param('userid') ) {
To
        if ( $type eq "opac" && $query->param('koha_login_context') && $query->param('koha_login_context') ne 'sco' && $query->param('password') && $query->param('userid') ) {

the problem will get fixed.
We did not work with libraries using SCO so far, so we did not take notice of that problem. With the opac-interface the koha_login_context is "opac". For SCO the koha_login_context is "sco".
Comment 16 Magnus Enger 2019-09-10 07:31:28 UTC Comment hidden (obsolete)
Comment 17 Owen Leonard 2019-09-10 16:10:29 UTC
Created attachment 92698 [details] [review]
Bug 22543: Prevent "back and refresh attack"

To reproduce and test:
- Log into the OPAC, you are taken to /cgi-bin/koha/opac-user.pl
- Log out, you are taken to /cgi-bin/koha/opac-main.pl?logout.x=1
- Click "Back", you are taken to /cgi-bin/koha/opac-user.pl
- Reload the page, you see an error like "Confirm new submission
  of form"
- Reload the page again and confirm the submission of the form
- You are now logged in to the OPAC again!
- Log out again
- Apply this patch
- Log in to the OPAC, you are taken to /cgi-bin/koha/opac-user.pl
- Log out, you are taken to /cgi-bin/koha/opac-main.pl?logout.x=1
- Click back, you are taken to /cgi-bin/koha/opac-user.pl
- No matter how many times you reload /cgi-bin/koha/opac-user.pl,
  you should not see anything other than the login form.
- Check that Self Check Out still works as it should, by making
  sure you have a user with self_check permissions, then setting
  WebBasedSelfCheck, AutoSelfCheckAllowed, AutoSelfCheckID and
  AutoSelfCheckPass appropriately. Then visit
  /cgi-bin/koha/sco/sco-main.pl and verify everything works as
  expected.

The messages and errors pages you see related to resubmitting the
form might differ from the ones described here, depending on what
browser you use. I tested in Chromium 76.0.x.

This fix was originally proposed by LMSCloud:
https://github.com/LMSCloud/Koha-LMSCloud/commit/74a7fe0f0c5b2ce0d65bd26452c6dcaf0a7f65ad

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 18 Marcel de Rooy 2019-09-12 13:32:05 UTC
Created attachment 92718 [details] [review]
Bug 22543: Prevent "back and refresh attack"

To reproduce and test:
- Log into the OPAC, you are taken to /cgi-bin/koha/opac-user.pl
- Log out, you are taken to /cgi-bin/koha/opac-main.pl?logout.x=1
- Click "Back", you are taken to /cgi-bin/koha/opac-user.pl
- Reload the page, you see an error like "Confirm new submission
  of form"
- Reload the page again and confirm the submission of the form
- You are now logged in to the OPAC again!
- Log out again
- Apply this patch
- Log in to the OPAC, you are taken to /cgi-bin/koha/opac-user.pl
- Log out, you are taken to /cgi-bin/koha/opac-main.pl?logout.x=1
- Click back, you are taken to /cgi-bin/koha/opac-user.pl
- No matter how many times you reload /cgi-bin/koha/opac-user.pl,
  you should not see anything other than the login form.
- Check that Self Check Out still works as it should, by making
  sure you have a user with self_check permissions, then setting
  WebBasedSelfCheck, AutoSelfCheckAllowed, AutoSelfCheckID and
  AutoSelfCheckPass appropriately. Then visit
  /cgi-bin/koha/sco/sco-main.pl and verify everything works as
  expected.

The messages and errors pages you see related to resubmitting the
form might differ from the ones described here, depending on what
browser you use. I tested in Chromium 76.0.x.

This fix was originally proposed by LMSCloud:
https://github.com/LMSCloud/Koha-LMSCloud/commit/74a7fe0f0c5b2ce0d65bd26452c6dcaf0a7f65ad

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 Marcel de Rooy 2019-09-12 13:35:48 UTC
QA Comment:
Looks good to me.
I was just thinking if this was a good moment too to ban logins via url parameters (via get, not post) too. Tested that too, still possible..
Comment 20 Fridolin Somers 2019-09-18 15:04:40 UTC
> - Check that Self Check Out still works as it should

What about Self Check In ? no impact on it ?
Comment 21 Magnus Enger 2019-09-19 08:28:28 UTC
(In reply to Fridolin SOMERS from comment #20)
> What about Self Check In ? no impact on it ?

SCI does not log in a staff user in the same way as SCO, and users can not log in, so as far as I can see there should be no impact.
Comment 22 Fridolin Somers 2019-09-19 11:24:13 UTC
(In reply to Magnus Enger from comment #21)
> (In reply to Fridolin SOMERS from comment #20)
> > What about Self Check In ? no impact on it ?
> 
> SCI does not log in a staff user in the same way as SCO, and users can not
> log in, so as far as I can see there should be no impact.

Ok thanks a lot Magnus.
Comment 23 Fridolin Somers 2019-09-19 11:26:20 UTC
Silly question :

> 1) Log into a Koh ausing URL parameters: ?userid=...&password=...&koha_login_context=opac

If you do that your password is in browser history no ?
Comment 24 Marcel de Rooy 2019-09-24 12:11:13 UTC
(In reply to Fridolin SOMERS from comment #23)
> Silly question :
> 
> > 1) Log into a Koh ausing URL parameters: ?userid=...&password=...&koha_login_context=opac
> 
> If you do that your password is in browser history no ?

Yes it is. And may be visible along the way too. So not very recommendable, but should we support such silly actions?
Comment 25 Liz Rea 2019-10-17 17:34:12 UTC
I've applied this to 18.05.x and can release at any time.
Comment 26 Lucas Gass 2019-11-22 19:00:30 UTC
this is applied to the 18.11.x security branch for 18.11.11
Comment 27 Martin Renvoize 2019-11-27 10:33:25 UTC
Nice work!

Pushed to master for 19.11.00
Comment 28 Jonathan Druart 2019-12-02 16:23:31 UTC
No tests here?
Comment 29 Jonathan Druart 2019-12-02 16:41:15 UTC
Two issues here:
1. Auth.t tests are failing:
t/db_dependent/Auth.t .. 1/22 Un-mocked method 'url()' called at /kohadevbox/koha/C4/Auth.pm line 1223.
Un-mocked method 'redirect()' called at /kohadevbox/koha/C4/Auth.pm line 1227.
A context appears to have been destroyed without first calling release().
Based on $@ it does not look like an exception was thrown (this is not always
a reliable test)

This is a problem because the global error variables ($!, $@, and $?) will
not be restored. In addition some release callbacks will not work properly from
inside a DESTROY method.

Here are the context creation details, just in case a tool forgot to call
release():
  File: t/db_dependent/Auth.t
  Line: 74
  Tool: Test::More::subtest

Cleaning up the CONTEXT stack...
# Test ended with extra hubs on the stack!
    # Looks like you planned 22 tests but ran 1.
t/db_dependent/Auth.t .. Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 21/22 subtests 

Test Summary Report
-------------------
t/db_dependent/Auth.t (Wstat: 65280 Tests: 1 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 22 tests but ran 1.
Files=1, Tests=1,  2 wallclock secs ( 0.02 usr  0.00 sys +  1.54 cusr  0.24 csys =  1.80 CPU)
Result: FAIL


The 2 unmock warnings are not related. The problem is that we are hitting an exit in C4::Auth

2. This code has been put before track_login_daily, is that expected?
Comment 30 Marcel de Rooy 2019-12-06 07:11:16 UTC
(In reply to Jonathan Druart from comment #29)
> 2. This code has been put before track_login_daily, is that expected?

No. The track call should be done before.
Comment 32 Katrin Fischer 2020-01-28 08:26:26 UTC
Hi David, please report a new security bug.
Comment 33 David Cook 2020-01-28 09:12:40 UTC
(In reply to Katrin Fischer from comment #32)
> Hi David, please report a new security bug.

You got it 👍
Comment 34 Marcel de Rooy 2020-08-11 11:21:43 UTC
(In reply to Marcel de Rooy from comment #30)
> (In reply to Jonathan Druart from comment #29)
> > 2. This code has been put before track_login_daily, is that expected?
> 
> No. The track call should be done before.

See bug 26191