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.
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?
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; }
(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.
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.
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
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.
It looks like LMSCloud has proposed a solution for this here: https://github.com/LMSCloud/Koha-LMSCloud/commit/5da5a0fb9259b371e15ef9b5b66253979e364859
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).
(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?
(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!
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
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.
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!
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).
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".
Created attachment 92679 [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
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>
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>
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..
> - Check that Self Check Out still works as it should What about Self Check In ? no impact on it ?
(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.
(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.
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 ?
(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?
I've applied this to 18.05.x and can release at any time.
this is applied to the 18.11.x security branch for 18.11.11
Nice work! Pushed to master for 19.11.00
No tests here?
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?
(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.
Hi David, please report a new security bug.
(In reply to Katrin Fischer from comment #32) > Hi David, please report a new security bug. You got it [U+1F44D]
(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