Skip to content

Commit

Permalink
Fix reCaptcha bypass
Browse files Browse the repository at this point in the history
Signed-off-by: Madhura Jayaratne <madhura.cj@gmail.com>
  • Loading branch information
madhuracj committed Sep 7, 2015
1 parent f81cc30 commit 0314e67
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 37 deletions.
3 changes: 3 additions & 0 deletions ChangeLog
@@ -1,6 +1,9 @@
phpMyAdmin - ChangeLog
======================

4.3.13.2 (Not yet released)
- bug [security] reCaptcha bypass

4.3.13.1 (2015-05-13)
- bug #4899 [security] CSRF vulnerability in setup
- bug #4900 [security] Vulnerability allowing man-in-the-middle attack
Expand Down
29 changes: 1 addition & 28 deletions libraries/plugins/auth/AuthenticationCookie.class.php
Expand Up @@ -218,18 +218,9 @@ public function auth()
. $GLOBALS['server'] . '" />';
} // end if (server choice)

// We already have one correct captcha.
$skip = false;
if ( isset($_SESSION['last_valid_captcha'])
&& $_SESSION['last_valid_captcha']
) {
$skip = true;
}

// Add captcha input field if reCaptcha is enabled
if ( !empty($GLOBALS['cfg']['CaptchaLoginPrivateKey'])
&& !empty($GLOBALS['cfg']['CaptchaLoginPublicKey'])
&& !$skip
) {
// If enabled show captcha to the user on the login screen.
echo '<script type="text/javascript">
Expand Down Expand Up @@ -349,18 +340,9 @@ public function authCheck()
return false;
}

// We already have one correct captcha.
$skip = false;
if ( isset($_SESSION['last_valid_captcha'])
&& $_SESSION['last_valid_captcha']
) {
$skip = true;
}

// Verify Captcha if it is required.
if ( !empty($GLOBALS['cfg']['CaptchaLoginPrivateKey'])
&& !empty($GLOBALS['cfg']['CaptchaLoginPublicKey'])
&& !$skip
) {
if ( !empty($_POST["recaptcha_challenge_field"])
&& !empty($_POST["recaptcha_response_field"])
Expand All @@ -378,22 +360,15 @@ public function authCheck()
// Check if the captcha entered is valid, if not stop the login.
if ( !$resp->is_valid ) {
$conn_error = __('Entered captcha is wrong, try again!');
$_SESSION['last_valid_captcha'] = false;
return false;
} else {
$_SESSION['last_valid_captcha'] = true;
}
} elseif (! empty($_POST["recaptcha_challenge_field"])
&& empty($_POST["recaptcha_response_field"])
) {
$conn_error = __('Please enter correct captcha!');
return false;
} else {
if (! isset($_SESSION['last_valid_captcha'])
|| ! $_SESSION['last_valid_captcha']
) {
return false;
}
return false;
}
}

Expand All @@ -406,8 +381,6 @@ public function authCheck()

if (! defined('TESTSUITE')) {
session_destroy();
// $_SESSION array is not immediately emptied
$_SESSION['last_valid_captcha'] = false;
}
// -> delete password cookie(s)
if ($GLOBALS['cfg']['LoginCookieDeleteAll']) {
Expand Down
19 changes: 10 additions & 9 deletions test/classes/plugin/auth/PMA_AuthenticationCookie_test.php
Expand Up @@ -186,7 +186,8 @@ public function testAuth()
$GLOBALS['cfg']['Lang'] = 'en';
$GLOBALS['cfg']['AllowArbitraryServer'] = true;
$GLOBALS['cfg']['Servers'] = array(1, 2);
$_SESSION['last_valid_captcha'] = true;
$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = '';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = '';
$GLOBALS['target'] = 'testTarget';
$GLOBALS['db'] = 'testDb';
$GLOBALS['table'] = 'testTable';
Expand Down Expand Up @@ -328,7 +329,6 @@ public function testAuth()
$GLOBALS['cfg']['Lang'] = '';
$GLOBALS['cfg']['AllowArbitraryServer'] = false;
$GLOBALS['cfg']['Servers'] = array(1);
$_SESSION['last_valid_captcha'] = false;
$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = 'testprivkey';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = 'testpubkey';
$GLOBALS['server'] = 0;
Expand Down Expand Up @@ -470,7 +470,6 @@ public function testAuthCheck()

// case 2

$_SESSION['last_valid_captcha'] = false;
$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = 'testprivkey';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = 'testpubkey';
$_POST["recaptcha_challenge_field"] = 'captcha1';
Expand All @@ -487,7 +486,6 @@ public function testAuthCheck()

// case 3

$_SESSION['last_valid_captcha'] = false;
$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = 'testprivkey';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = 'testpubkey';
$_POST["recaptcha_challenge_field"] = '';
Expand Down Expand Up @@ -532,7 +530,8 @@ public function testAuthCheck()

// case 6

$_SESSION['last_valid_captcha'] = true;
$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = '';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = '';
$_REQUEST['old_usr'] = '';
$_REQUEST['pma_username'] = 'testPMAUser';
$_REQUEST['pma_servername'] = 'testPMAServer';
Expand Down Expand Up @@ -662,8 +661,8 @@ public function testAuthCheckDecryptUser()
$_COOKIE['pma_iv'] = base64_encode('testiv09testiv09');
$GLOBALS['cfg']['blowfish_secret'] = 'secret';
$_SESSION['last_access_time'] = '';
$_SESSION['last_valid_captcha'] = true;

$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = '';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = '';
// mock for blowfish function
$this->object = $this->getMockBuilder('AuthenticationCookie')
->disableOriginalConstructor()
Expand Down Expand Up @@ -700,7 +699,8 @@ public function testAuthCheckDecryptPassword()
$_COOKIE['pmaPass-1'] = 'pmaPass1';
$_COOKIE['pma_iv'] = base64_encode('testiv09testiv09');
$GLOBALS['cfg']['blowfish_secret'] = 'secret';
$_SESSION['last_valid_captcha'] = true;
$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = '';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = '';
$_SESSION['last_access_time'] = time() - 1000;
$GLOBALS['cfg']['LoginCookieValidity'] = 1440;

Expand Down Expand Up @@ -745,7 +745,8 @@ public function testAuthCheckAuthFails()
$_COOKIE['pma_iv'] = base64_encode('testiv09testiv09');
$GLOBALS['cfg']['blowfish_secret'] = 'secret';
$_SESSION['last_access_time'] = 1;
$_SESSION['last_valid_captcha'] = true;
$GLOBALS['cfg']['CaptchaLoginPrivateKey'] = '';
$GLOBALS['cfg']['CaptchaLoginPublicKey'] = '';
$GLOBALS['cfg']['LoginCookieValidity'] = 0;
$_SESSION['last_access_time'] = -1;
// mock for blowfish function
Expand Down

0 comments on commit 0314e67

Please sign in to comment.