View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0017984 | mantisbt | security | public | 2014-12-24 06:18 | 2015-01-27 04:50 |
Reporter | nextgens | Assigned To | dregad | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.2.18 | ||||
Target Version | 1.2.19 | Fixed in Version | 1.2.19 | ||
Summary | 0017984: CVE-2014-9624: CAPTCHA bypass is way easier than it should be | ||||
Description | One can get an unlimited amount of "samples" with different perturbations for the same challenge... which makes the whole captcha utterly useless and very easy to bypass. Solutions involve either caching the captcha or changing the challenge | ||||
Steps To Reproduce | 1) load the following in a browser tab 2) load this in a new tab 3) refresh the second tab until you're 100% confident your poor OCR got it right | ||||
Tags | No tags attached. | ||||
Attached Files | 0001-Generate-a-unique-CAPTCHA-for-a-given-private-key.patch (3,442 bytes)
From f87632f56c16f80fec532f13e7624c315e562063 Mon Sep 17 00:00:00 2001 From: Damien Regad <dregad@mantisbt.org> Date: Mon, 29 Dec 2014 23:05:05 +0100 Subject: [PATCH 1/3] Generate a unique CAPTCHA for a given private key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prior to this, the code would be the same but the displayed CAPTCHA would vary slightly with each call to make_captcha_img.php, allowing an attacker unlimited attempts to train their OCR software to decode the image. By seeding the random number generator, we ensure that the generated CAPTCHA is always the same for a given private key. The commit also removes several unnecessary calls to srand(). This issue was reported by Florent Daignière (nextgens). Fixes #17984 --- make_captcha_img.php | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/make_captcha_img.php b/make_captcha_img.php index 2bf3734..28391d3 100644 --- a/make_captcha_img.php +++ b/make_captcha_img.php @@ -149,6 +149,8 @@ function make_captcha( $private_key ) { + srand( session_get( CAPTCHA_KEY ) ); + if($this->debug) echo "\n<br />-Captcha-Debug: Generate private key: ($private_key)"; // create Image and set the apropriate function depending on GD-Version & websafecolor-value @@ -182,17 +184,12 @@ if($this->debug) echo "\n<br />-Captcha-Debug: Fill background with noise: (".$this->nb_noise.")"; for($i=0; $i < $this->nb_noise; $i++) { - srand((double)microtime()*1000000); $size = intval(rand((int)($this->minsize / 2.3), (int)($this->maxsize / 1.7))); - srand((double)microtime()*1000000); $angle = intval(rand(0, 360)); - srand((double)microtime()*1000000); $x = intval(rand(0, $this->lx)); - srand((double)microtime()*1000000); $y = intval(rand(0, (int)($this->ly - ($size / 5)))); $this->random_color(160, 224); $color = $func2($image, $this->r, $this->g, $this->b); - srand((double)microtime()*1000000); $text = chr(intval(rand(45,250))); if(count ($this->TTF_RANGE)>0){ @ImageTTFText($image, $size, $angle, $x, $y, $color, $this->change_TTF(), $text); @@ -225,11 +222,8 @@ for($i=0, $x = intval(rand($this->minsize,$this->maxsize)); $i < $this->chars; $i++) { $text = utf8_strtoupper(substr($private_key, $i, 1)); - srand((double)microtime()*1000000); $angle = intval(rand(($this->maxrotation * -1), $this->maxrotation)); - srand((double)microtime()*1000000); $size = intval(rand($this->minsize, $this->maxsize)); - srand((double)microtime()*1000000); $y = intval(rand((int)($size * 1.5), (int)($this->ly - ($size / 7)))); $this->random_color(0, 127); $color = $func2($image, $this->r, $this->g, $this->b); @@ -270,11 +264,8 @@ function random_color($min,$max) { - srand((double)microtime() * 1000000); $this->r = intval(rand($min,$max)); - srand((double)microtime() * 1000000); $this->g = intval(rand($min,$max)); - srand((double)microtime() * 1000000); $this->b = intval(rand($min,$max)); } @@ -283,7 +274,6 @@ if(count($this->TTF_RANGE) > 0){ if(is_array($this->TTF_RANGE)) { - srand((float)microtime() * 10000000); $key = array_rand($this->TTF_RANGE); $this->TTF_file = $this->TTF_folder.$this->TTF_RANGE[$key]; } -- 1.9.1 0002-Cache-generated-captcha-to-ensure-uniqueness.patch (4,391 bytes)
From 8b23a8163273bcf9bb87d402d0691c81cc1bb7d8 Mon Sep 17 00:00:00 2001 From: Damien Regad <dregad@mantisbt.org> Date: Fri, 2 Jan 2015 01:39:14 +0100 Subject: [PATCH] Cache generated captcha to ensure uniqueness This is an improvement over the earlier fix which seeded the random number generator with the captcha's key. As Florent pointed out, the cure was worse than the disease as it reduced the effective number of distinct captchas to a mere 2^31, making it easy for an attacker to precompute the lot to bypass the challenge. In addition, debug mode now works in context of make_captcha_img.php, i.e. it doesn't set the image/jpeg content type header, and displays the generated captcha as an image within an html page. Fixes #17984 --- core/constant_inc.php | 1 + make_captcha_img.php | 40 +++++++++++++++++++++++++++++++++++----- signup.php | 3 +++ signup_page.php | 1 + 4 files changed, 40 insertions(+), 5 deletions(-) diff --git a/core/constant_inc.php b/core/constant_inc.php index 25715ca..7e04801 100644 --- a/core/constant_inc.php +++ b/core/constant_inc.php @@ -542,3 +542,4 @@ define( 'PASSWORD_MAX_SIZE_BEFORE_HASH', 1024 ); define( 'SECONDS_PER_DAY', 86400 ); define( 'CAPTCHA_KEY', 'captcha_key' ); +define( 'CAPTCHA_IMG', 'captcha_image' ); diff --git a/make_captcha_img.php b/make_captcha_img.php index 28391d3..cda9b29 100644 --- a/make_captcha_img.php +++ b/make_captcha_img.php @@ -147,10 +147,8 @@ if($this->debug) echo "\n<br />-Captcha-Debug: Set image dimension to: (".$this->lx." x ".$this->ly.")"; } - function make_captcha( $private_key ) + function generate_captcha( $private_key ) { - srand( session_get( CAPTCHA_KEY ) ); - if($this->debug) echo "\n<br />-Captcha-Debug: Generate private key: ($private_key)"; // create Image and set the apropriate function depending on GD-Version & websafecolor-value @@ -218,7 +216,7 @@ } // generate Text - if($this->debug) echo "\n<br />-Captcha-Debug: Fill forground with chars and shadows: (".$this->chars.")"; + if($this->debug) echo "\n<br />-Captcha-Debug: Fill foreground with chars and shadows: (".$this->chars.")"; for($i=0, $x = intval(rand($this->minsize,$this->maxsize)); $i < $this->chars; $i++) { $text = utf8_strtoupper(substr($private_key, $i, 1)); @@ -239,10 +237,42 @@ } $x += (int)($size + ($this->minsize / 5)); } - header('Content-type: image/jpeg'); + + # Generate the JPEG + ob_start(); @ImageJPEG($image, null, $this->jpegquality); + $jpg = ob_get_contents(); + ob_end_clean(); + @ImageDestroy($image); if($this->debug) echo "\n<br />-Captcha-Debug: Destroy Imagestream."; + + return $jpg; + } + + function make_captcha( $private_key ) + { + # Retrieve previously image generated from session cache + $t_image = session_get( CAPTCHA_IMG, null ); + + if( is_null( $t_image ) ) { + $t_image = $this->generate_captcha( $private_key ); + if( $this->debug ) { + echo "\n<br />-Captcha-Debug: Caching generated image."; + } + session_set( CAPTCHA_IMG, $t_image ); + } elseif( $this->debug ) { + echo "\n<br />-Captcha-Debug: Retrieved image from cache."; + } + + # Output + if( $this->debug ) { + echo "\n<br />-Captcha-Debug: Generated image (" . strlen( $t_image ) . " bytes): " + . '<img src="data:image/jpeg;base64,' . base64_encode( $t_image ) . '">'; + } else { + header('Content-type: image/jpeg'); + echo $t_image; + } } /** @private **/ diff --git a/signup.php b/signup.php index b63e772..8ee2449 100644 --- a/signup.php +++ b/signup.php @@ -63,6 +63,9 @@ if ( $t_key != $f_captcha ) { trigger_error( ERROR_SIGNUP_NOT_MATCHING_CAPTCHA, ERROR ); } + + # Clear captcha cache + session_delete( CAPTCHA_IMG ); } email_ensure_not_disposable( $f_email ); diff --git a/signup_page.php b/signup_page.php index 3a8c725..b7a302c 100644 --- a/signup_page.php +++ b/signup_page.php @@ -67,6 +67,7 @@ $t_allow_passwd = helper_call_custom_function( 'auth_can_change_password', array() ); if( ON == config_get( 'signup_use_captcha' ) && get_gd_version() > 0 && ( true == $t_allow_passwd ) ) { session_set( CAPTCHA_KEY, mt_rand() ); + session_delete( CAPTCHA_IMG ); # captcha image requires GD library and related option to ON ?> -- 1.9.1 | ||||
Spending time fixing this captcha kind of feels like beating on a dead horse, but anyway here it goes... please review the attached patch, let me know your feedback. How would you like to be credited for this finding ? |
|
Hmm, the patch addresses the problem at hand but highlights another vulnerability... unless I'm mistaken, the MT-based RNG is now seeded once, only with an int (in signup_page.php)... which means there's only "an int worth" (~4billions on 32bit) of possibilities (as opposed to the same keyspace but lots of different permutations before the patch). One can pre-compute the whole lot and instantly lookup the challenge... The proposed cure looks worst than the disease in this case... Please credit "Florent Daigniere from Matta Consulting" |
|
That's ~2m (2^31) different captcha keys actually, but that's not different from before (and already an improvement over Mantis < 1.2.18 where the range was only 0..9999).
I didn't think about that. Seeding the RNG sounded like a simple and effective approach, but I guess it was just too easy ;-) The only alternative I can think of ATM (short of changing the whole captcha code, which is not worth the effort as we're switching to a new one in 1.3 anyway) would be to cache the generated image in the user's session. Thoughts ? |
|
Caching seems like the right solution to me |
|
Salut Florent, Attached is a 2nd patch (to be applied on top of the previous one), which implements caching. As I was testing this code, I noticed a regression bug in the 1.2.18 captcha validation, which was also reported by another user (see 0017993), you might want to check out the fix in the proposed pull request [1] too (feedback welcome). |
|
I would move the session_delete before the trigger_error() call in signup.php ... otherwise there's a race-condition the attacker is sure to win (he can read the response slowly and retry against the same challenge in parallel) As for the session handling/pull request, I'm not sure I understand why the session is trashed in the first place. If what you're trying to avoid is session fixation attacks, changing the session identifier is the only thing that needs to happen... there's no need to void the content of the session itself (that's what session_regenerate_id() does) |
|
Thanks for your feedback.
Makes sense, will do.
I'm not sure either, tbh... Never touched this code before. My understanding is that signing up implies logging out of Mantis, which is necessary because when signing up, someone may currently be logged in, either as anonymous or less likely as themselves (in which case signup_page.php needs to be accessed manually). I think it makes sense to clear the session's contents when logging out. Does that make sense to you ? |
|
Destroying the session when logging out, sure... For the rest not really. Traditionally applications allow concurrent sessions for users... anonymous/guest sessions are handled the same way... and that allows for "upgrading" an unauthenticated session to an authenticated one (sessions are independent so there's no problem there). I suspect that Mantis sessions aren't handled that way... whether it makes sense to change it for this stable branch I'm not sure ^-^ |
|
CVE request sent http://article.gmane.org/gmane.comp.security.oss.general/15434 |
|
MITRE assigned CVE-2014-9624 to this issue [1]. [1] http://article.gmane.org/gmane.comp.security.oss.general/15452 |
|
MantisBT: master-1.2.x 39a92726 2014-12-29 12:05 Details Diff |
Cache the generated CAPTCHA to ensure uniqueness Prior to this, the code would be the same but the displayed CAPTCHA would vary slightly with each call to make_captcha_img.php, allowing an attacker unlimited attempts to train their OCR software to decode the image. This issue was reported by Florent Daignière from Matta Consulting. In addition, CAPTCHA debug mode now works in make_captcha_img.php's context, i.e. it doesn't set the image/jpeg content type header, and displays the generated captcha as an image within an html page. Fixes 0017984 |
Affected Issues 0017984 |
|
mod - core/constant_inc.php | Diff File | ||
mod - make_captcha_img.php | Diff File | ||
mod - signup.php | Diff File | ||
mod - signup_page.php | Diff File |