Attachment 61110 Details for Bug 18275 – Bug 18275: Do not rely on CGI param userid to log a user in if auth is not required

[patch] Bug 18275: Do not rely on CGI param userid to log a user in if auth is not required

Bug-18275-Do-not-rely-on-CGI-param-userid-to-log-a.patch (text/plain), 2.79 KB, created by Jonathan Druart on 2017-03-15 18:00:32 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Druart

Created: 2017-03-15 18:00:32 UTC

Size: 2.79 KB

patch

obsolete

>From 7c0bce60f16699df83935c00bfbb9d441b043ee3 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Wed, 15 Mar 2017 14:52:49 -0300
>Subject: [PATCH] Bug 18275: Do not rely on CGI param userid to log a user in
> if auth is not required
>
>From opac/opac-memberentry.pl, authnotrequired is set.
>That means a patron can access the page without being logged in. It is
>used on this page for the self registration feature.
>
>From C4::Auth::get_template_and_user, we have
>  $userid = $q_userid;
>$q_userid is previously set to the 'userid' CGI param.
>
>We end up here if authonotrequired is set AND CGISESSID does not exist.
>
>Test plan:
>Do not help script kiddies...
>---
> C4/Auth.pm | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/C4/Auth.pm b/C4/Auth.pm
>index 20d64a5..5dcf1cd 100644
>--- a/C4/Auth.pm
>+++ b/C4/Auth.pm
>@@ -914,14 +914,13 @@ sub checkauth {
>             -value    => $session->id,
>             -HttpOnly => 1
>         );
>-        $userid = $q_userid;
>         my $pki_field = C4::Context->preference('AllowPKIAuth');
>         if ( !defined($pki_field) ) {
>             print STDERR "ERROR: Missing system preference AllowPKIAuth.\n";
>             $pki_field = 'None';
>         }
>         if ( ( $cas && $query->param('ticket') )
>-            || $userid
>+            || $q_userid
>             || ( $shib && $shib_login )
>             || $pki_field ne 'None' )
>         {
>@@ -935,7 +934,7 @@ sub checkauth {
>                 my $retuserid;
> 
>                 # Do not pass password here, else shib will not be checked in checkpw.
>-                ( $return, $cardnumber, $retuserid ) = checkpw( $dbh, $userid, undef, $query );
>+                ( $return, $cardnumber, $retuserid ) = checkpw( $dbh, $q_userid, undef, $query );
>                 $userid      = $retuserid;
>                 $shibSuccess = $return;
>                 $info{'invalidShibLogin'} = 1 unless ($return);
>@@ -986,7 +985,7 @@ sub checkauth {
>                 else {
>                     my $retuserid;
>                     ( $return, $cardnumber, $retuserid ) =
>-                      checkpw( $dbh, $userid, $password, $query, $type );
>+                      checkpw( $dbh, $q_userid, $password, $query, $type );
>                     $userid = $retuserid if ($retuserid);
>                     $info{'invalid_username_or_password'} = 1 unless ($return);
>                 }
>@@ -1131,7 +1130,7 @@ sub checkauth {
>                 $session->param( 'ip',       $session->remote_addr() );
>                 $session->param( 'sessiontype', 'anon' );
>             }
>-        }    # END if ( $userid    = $query->param('userid') )
>+        }    # END if ( $q_userid
>         elsif ( $type eq "opac" ) {
> 
>             # if we are here this is an anonymous session; add public lists to it and a few other items...
>-- 
>2.9.3

Actions: View | Diff | Splinter Review

Attachments on bug 18275: 61110 | 61113 | 61114 | 61118 | 61167 | 61168