View Issue Details

IDProjectCategoryView StatusLast Update
0017811mantisbtsecuritypublic2015-01-03 16:24
Reporteralex91ar Assigned Tovboctor  
PriorityhighSeveritymajorReproducibilityalways
Status closedResolutionfixed 
Product Version1.2.17 
Target Version1.2.18Fixed in Version1.2.18 
Summary0017811: CVE-2014-9117: CAPTCHA bypass
Description

I recently stumbled upon your software and decided to download a copy of it and perform some pentesting on it.

I must say it performs very well and I would definitely use it to track issues for my project, it's simple to use and the interface is quite intuitive.
So in order to better improve this software, I present you my findings:

  • Open redirect: A malicious individual could make a user click on an specially crafted URL that would redirect the user to an arbitrary website. (Open redirect: http://cwe.mitre.org/data/definitions/601.html)

  • Captcha bypass: There is a weakness on the CAPTCHA system that is used upon registration of a new user that could allow a malicious individual to perform a denial of service by indiscriminately creating new accounts, thus generating a high load on the server.

As a security professional, I am instructed to notice potential vectors for security issues, hence my curiosity when I saw these occurrences.

Please acknowledge these bugs so I can further assist you in reproducing and fixing these issues.

Best regards,
Alejo Popovici

TagsNo tags attached.
Attached Files
Screens.zip (215,557 bytes)
0001-Use-session-rather-than-form-key-for-captcha.patch (3,400 bytes)   
From 882c7a75975752a3bf86554eba0bfd290d5dc154 Mon Sep 17 00:00:00 2001
From: Victor Boctor <victor@mantishub.net>
Date: Mon, 24 Nov 2014 20:28:34 -0800
Subject: [PATCH] Use session rather than form key for captcha

Fixes #17811
---
 core/constant_inc.php | 4 ++++
 make_captcha_img.php  | 4 ++--
 signup.php            | 5 +++--
 signup_page.php       | 7 +++----
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/core/constant_inc.php b/core/constant_inc.php
index 579bf81..e0c1c0d 100644
--- a/core/constant_inc.php
+++ b/core/constant_inc.php
@@ -540,3 +540,7 @@ define( 'DB_FIELD_SIZE_PASSWORD', 32);
 define( 'PASSWORD_MAX_SIZE_BEFORE_HASH', 1024 );
 
 define( 'SECONDS_PER_DAY', 86400 );
+
+define( 'CAPTCHA_KEY', 'captcha_key' );
+
+
diff --git a/make_captcha_img.php b/make_captcha_img.php
index 414e4a3..2bf3734 100644
--- a/make_captcha_img.php
+++ b/make_captcha_img.php
@@ -26,9 +26,9 @@
 	  */
 	require_once( 'core.php' );
 
-	$f_public_key = gpc_get_int( 'public_key' );
+	$t_form_key = session_get( CAPTCHA_KEY );
 
-	$t_key = utf8_strtolower( utf8_substr( md5( config_get( 'password_confirm_hash_magic_string' ) . $f_public_key ), 1, 5) );
+	$t_key = utf8_strtolower( utf8_substr( md5( config_get( 'password_confirm_hash_magic_string' ) . $t_form_key ), 1, 5) );
 	$t_system_font_folder = get_font_path();
 	$t_font_per_captcha = config_get( 'font_per_captcha' );
 
diff --git a/signup.php b/signup.php
index 486181a..37f3f27 100644
--- a/signup.php
+++ b/signup.php
@@ -32,7 +32,6 @@
 	$f_username		= strip_tags( gpc_get_string( 'username' ) );
 	$f_email		= strip_tags( gpc_get_string( 'email' ) );
 	$f_captcha		= gpc_get_string( 'captcha', '' );
-	$f_public_key	= gpc_get_int( 'public_key', '' );
 
 	$f_username = trim( $f_username );
 	$f_email = email_append_domain( trim( $f_email ) );
@@ -51,8 +50,10 @@
 
 	if( ON == config_get( 'signup_use_captcha' ) && get_gd_version() > 0 	&&
 				helper_call_custom_function( 'auth_can_change_password', array() ) ) {
+		$t_form_key = session_get( CAPTCHA_KEY );
+
 		# captcha image requires GD library and related option to ON
-		$t_key = utf8_strtolower( utf8_substr( md5( config_get( 'password_confirm_hash_magic_string' ) . $f_public_key ), 1, 5) );
+		$t_key = utf8_strtolower( utf8_substr( md5( config_get( 'password_confirm_hash_magic_string' ) . $t_form_key ), 1, 5) );
 
 		if ( $t_key != $f_captcha ) {
 			trigger_error( ERROR_SIGNUP_NOT_MATCHING_CAPTCHA, ERROR );
diff --git a/signup_page.php b/signup_page.php
index 9387611..784809a 100644
--- a/signup_page.php
+++ b/signup_page.php
@@ -35,8 +35,6 @@
 
 	html_page_top1();
 	html_page_top2a();
-
-	$t_key = mt_rand( 0,99999 );
 ?>
 
 <br />
@@ -68,6 +66,8 @@
 <?php
 	$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( 0,99999 ) );
+
 		# captcha image requires GD library and related option to ON
 ?>
 <tr class="row-1">
@@ -78,8 +78,7 @@
 		<?php print_captcha_input( 'captcha', '' ) ?>
 	</td>
 	<td>
-		<img src="make_captcha_img.php?public_key=<?php echo $t_key ?>" alt="visual captcha" />
-		<input type="hidden" name="public_key" value="<?php echo $t_key ?>" />
+		<img src="make_captcha_img.php" alt="visual captcha" />
 	</td>
 </tr>
 <?php
-- 
1.9.3 (Apple Git-50)

Relationships

related to 0017648 closeddregad CVE-2014-6316: URL redirection issue 
related to 0017984 closeddregad CVE-2014-9624: CAPTCHA bypass is way easier than it should be 
related to 0017993 closeddregad User creation with captcha broken by fix for issue 0017811 

Activities

dregad

dregad

2014-10-28 08:49

developer   ~0041692

Greetings,

Thanks for your interest in Mantis and for bringing these issues to our attention.

  1. sounds like an issue we're already aware of
  2. seems to be something new

I'd appreciate if you could post further details here, including detailed steps to reproduce if possible as well as a patch should you have one (attached as unified diff).

Also, should we confirm the issues and subsequently open CVEs for them, let us know if you'd like to be credited for the finding(s) and if so, how you'd like your name to appear.

alex91ar

alex91ar

2014-10-28 19:46

reporter   ~0041697

Hi there,
I uploaded screenshots showing the issues.
This is a brief how to that will enable to you reproduce the issues:
1) Open Redirect: This link will redirect you to google: https://www.mantisbt.org/bugs/login_page.php?return=http:/www.google.com

2) Captcha Bypass:
a) First we start by finding out the output for a captcha. With this we know that the captcha produced by the following link with public_key=0: https://www.mantisbt.org/bugs/make_captcha_img.php?public_key=0 produces always the captcha "E4652".
b) When creating a user, the request sends the public_key with the answer to the captcha. This can be modified so that always the same captcha is used. A request would look like the screenshot I uploaded.

Possible solutions:
I'm not skilled enough in PHP to propose a definite solution, but I can give a general idea of the best practices.
1) In "String_api.php" on the function string_sanitize_url, line 253, modify the pattern recognition or the algorithm so that it matches ":/". The best thing would be to create a whitelist that would only redirect to files present on the webapp folder.

2) The best thing to do is to implement ReCaptcha, it's being used widely and it shows the best success rates against automated attacks. But a possible workaround would be to store on the server-side the public_key instead of passing it as a parameter.

Also, as a sidenote, I noticed that on the database the passwords are stored as an md5 hash. MD5 is now deprecated and newer standards like SHA-256 should be implemented. Also, it could be considered the possibility of adding a salt to the hashed password, thus preventing pre-computed bruteforce attacks. It's not critical but it would make it harder to gain access were the database compromised.

Regarding the credit, I would very much like to be credited for the findings with my full name on it which is "Alejo Popovici".

Finally, I'd like to take a chance to thank you for the hasty response, not everybody thinks that security is something that should not be taken lightly.

I'll keep testing your app and let you know if I find anything interesting.
Best regards,
Alex.

dregad

dregad

2014-10-29 08:47

developer   ~0041701

Hi Alex,

Many thanks for your feedback.

  1. is definitely the same issue we have already logged: 0017648 (CVE-2014-6316), follow-up on resolution will take place there. As it's currently a private issue, you won't be able to see it but you should be notified when it gets resolved.

Moving forward, this issue will therefore only be used for tracking of the captcha vulnerability. Have you requested a CVE for this already ? Give me the number if you did; if not I can take care of it.

With regards to the MD5 hashing os passwords, this in an issue we have been aware of for a long time (see 0011250, 0010172 and to a lesser extent 0012957). We do plan to fix this in the future, although IMO we should use crypt() and not than SHA which is not a good method for hashing passwords. We definitely would add a salt as part of this process. I suggest you monitor these issues to be informed of any progress made.

dregad

dregad

2014-10-29 08:54

developer   ~0041702

Git blame says the captcha code was introduced in 1.0.0, which means this vulnerability exists since 2004 (!)

alex91ar

alex91ar

2014-10-29 09:00

reporter   ~0041703

Hi dregad,
I don't have a CVE for this issue, this is going to be my first CVE, and I didn't want to make it public until the issue was fixed.

I'm glad to see that you guys are taking precautions into upgrading your security to follow the latest standards.

Thanks for everything!
Best regards,
Alex.

atrol

atrol

2014-10-29 10:48

developer   ~0041704

Last edited: 2014-10-29 10:49

we should use crypt()

Didn't have a deeper look, but we use crypt() with login_method CRYPT
https://github.com/mantisbt/mantisbt/blob/master/core/authentication_api.php#L476
Adding $g_crypto_master_salt as a parameter to calls of function auth_process_plain_password would also add a static salt.

dregad

dregad

2014-10-29 11:38

developer   ~0041705

we use crypt() with login_method CRYPT

Right, but we still default to MD5 (this was changed in 2003, commit 65d40c2558dafa0ce1810673c29a6bdcbef9638f, I guess due to a vulnerability in crypt() at the time).

Although I guess it would still be better than no salt at all, using a static salt is not good enough because an attacker could build a rainbow table to crack all passwords in parallel (instead of just one).

Also worth mentioning that $g_crypto_master_salt does not exist in 1.2.x branch.

dregad

dregad

2014-10-29 11:46

developer   ~0041706

One more thing: as per [1], I think the target should be the new Password Hashing library [2] available in PHP 5.5, and use the compatibility library [3] for older versions.

Moreover, since we're also discussing 1.2 here, it's worth mentioning that in PHP < 5.3 crypt() relies on algorithms available in the system, so the stronger ones (e.g. Blowfish) may not be available.

[1] http://php.net/manual/en/faq.passwords.php#faq.passwords.bestpractice
[2] http://php.net/manual/en/book.password.php
[3] https://github.com/ircmaxell/password_compat (requires PHP 5.3.7 though)

alex91ar

alex91ar

2014-11-03 12:07

reporter   ~0041757

Hi!
Please let me know as soon as you have a CVE number assigned.
Thanks!
Alex.

vboctor

vboctor

2014-11-24 23:30

manager   ~0041898

I've attached a patch [1] to use php session rather than form fields to communicate the key between form page, image page, and action page.

Since we use a new captcha in master (i.e. 1.3.x), I expect that this issue is only targeted for master-1.2.x branch.

[1] 0001-Use-session-rather-than-form-key-for-captcha.patch

dregad

dregad

2014-11-25 17:57

developer   ~0041916

Alex, can you please confirm that Victor's patch indeed fixes the issue (it does, AFAICT).

Thanks in advance for your feedback.

alex91ar

alex91ar

2014-11-25 20:12

reporter   ~0041918

Hi,
To my knowledge, it should fix the issue, although in order to stick to the best practices, I would recommend to increase the entropy for the random number (from 99999 to 9999999 for example), so that we take away the possibility of a pre-computed captcha table. It's not a high priority issue, but if it can be increased without any major changes, it would be best.

Thanks and best regards,
Alex.

dregad

dregad

2014-11-26 11:53

developer   ~0041921

I would recommend to increase the entropy for the random number (from 99999 to 9999999 for example)

That shouldn't be an issue. I'll remove the bounds so we get a number between 0 and mt_getrandmax(). Thanks for the hint.

alex91ar

alex91ar

2014-11-26 12:02

reporter   ~0041922

Hi Damien, great job on the fix.
Are you submitting the CVE request?
Thanks!
Best regards,
Alex.

dregad

dregad

2014-11-26 12:07

developer   ~0041923

Last edited: 2014-11-26 12:11

CVE request sent: http://thread.gmane.org/gmane.comp.security.oss.general/14906

EDIT:

Are you submitting the CVE request?

I was actually doing this as you were posting ;)

Related Changesets

MantisBT: master-1.2.x 7bb78e45

2014-11-24 18:28

vboctor

Committer: dregad


Details Diff
Use session rather than form key for captcha

Fixes 0017811

Signed-off-by: Damien Regad <dregad@mantisbt.org>
Affected Issues
0017811, 0017993
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

MantisBT: master-1.2.x b1a6ee2c

2014-11-26 06:05

dregad


Details Diff
Increase captcha public key max value

The captcha's public key was generated as a random number between 0 and
99999.

As per Alejo Popovici's recommendation in 0017811:0041918, this commit removes
the limitation in mt_rand() call, so the generated key is now a number
between 0 and mt_getrandmax() (2147483647 on my box).

Issue 0017811
Affected Issues
0017811
mod - signup_page.php Diff File