View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0017811 | mantisbt | security | public | 2014-10-28 08:40 | 2015-01-03 16:24 |
Reporter | alex91ar | Assigned To | vboctor | ||
Priority | high | Severity | major | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.2.17 | ||||
Target Version | 1.2.18 | Fixed in Version | 1.2.18 | ||
Summary | 0017811: 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.
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, | ||||
Tags | No tags attached. | ||||
Attached Files | 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) | ||||
Greetings, Thanks for your interest in Mantis and for bringing these issues to our attention.
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. |
|
Hi there, 2) Captcha Bypass: Possible solutions: 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. |
|
Hi Alex, Many thanks for your feedback.
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. |
|
Git blame says the captcha code was introduced in 1.0.0, which means this vulnerability exists since 2004 (!) |
|
Hi dregad, I'm glad to see that you guys are taking precautions into upgrading your security to follow the latest standards. Thanks for everything! |
|
Didn't have a deeper look, but 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. |
|
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 |
|
Hi! |
|
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 |
|
Alex, can you please confirm that Victor's patch indeed fixes the issue (it does, AFAICT). Thanks in advance for your feedback. |
|
Hi, Thanks and best regards, |
|
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. |
|
Hi Damien, great job on the fix. |
|
CVE request sent: http://thread.gmane.org/gmane.comp.security.oss.general/14906 EDIT:
I was actually doing this as you were posting ;) |
|
MantisBT: master-1.2.x 7bb78e45 2014-11-24 18:28 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 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 |