ldap authentication: allow login with null byte - null byte attack

Thanks. I think the fix is:

- make sure all calls to ->bind that are anonymous are explicitly anonymous: LdapServer::bind(NULL,NULL,TRUE);
- make sure all other calls to bind have a dn and password >0 length

I'm running the simpletests now to see if it breaks anything. Attached is the patch. Teh same basic changes will apply to the 7.x-1.x-dev branch.

Should we move this? https://drupal.org/node/101494

Thanks Erik. I did this and am working on the 7.x-1.x-dev patch. I'm sending you a copy of what I sent them. Its critical that anyone that can reproduce this on 7.x-2.x-dev before the patch test the patch. It needs to be those that have ldaps that allow anonymous binds. The patch http://drupalcode.org/project/ldap.git/commitdiff/22db40d7a987981fa5f3d7... was February 19th.

@erikwebb - http://drupal.org/security-advisory-policy this module doesn't have any stable releases so it doesn't need to be handled in private.

I committed a very similar patch (http://drupalcode.org/project/ldap.git/commitdiff/88f3c46dd3719bb6dbd3cb...) to the 7.x-1.x-dev branch.

Note that #1857606: LDAP Authentication: login with empty password creates user session is related to the fix for this, even though it doesn't involve null bytes.

This inspired me to write a simple module that filters all form input from NULL byte poisoning.
If anyone would be so kind to have a look at it :)

Shameless plug

beautifully simple and done module. thanks.

LDAP authentication fails for my sites after this commit.
I get "LDAP bind failure for user userdn=, pass=." in the logs. The test login in the server configuration page still works.

This was on the 7.x-1.x-dev branch.

which ldap authentication method are you using for binding?

I'm using the "Anonymous Bind for search, then Bind with Users Credentials" method.

John, I think this might be the problem. In your patch to the 7.x-1.x branch you changed ldap_authentication.inc

@@ -260,7 +260,7 @@ function _ldap_authentication_user_login_authenticate_validate(&$form_state) {
$transformname = $ldap_server->drupalToLdapNameTransform($authname, $watchdog_tokens);
$replace = array($basedn, $transformname);
$userdn = str_replace($search, $replace, $ldap_server->user_dn_expression);
- $bind_success = ($ldap_server->bind($userdn, $pass) == LDAP_SUCCESS);
+ $bind_success = ($ldap_server->bind($userdn, $password, FALSE) == LDAP_SUCCESS);
if ($bind_success) {
break;
}

You substituted $password for $pass.

While I think the previous post highlights a coding error. It is not the cause of my inability to authenticate. I had a close look and it appears to be here in ldap_authentication.inc

 243     // #2 BIND TO SERVER
 244     $bind_success = FALSE;
 245     if ($ldap_server->bind_method == LDAP_SERVERS_BIND_METHOD_SERVICE_ACCT ||
 246         $ldap_server->bind_method == LDAP_SERVERS_BIND_METHOD_ANON_USER
 247         ) {
 248       $bind_success = ($ldap_server->bind(NULL, NULL, FALSE) == LDAP_SUCCESS);
 249     }
 250     elseif ($ldap_server->bind_method == LDAP_SERVERS_BIND_METHOD_ANON) {
 251       $bind_success = ($ldap_server->bind(NULL, NULL, TRUE) == LDAP_SUCCESS);
 252     }

Moving the test for LDAP_SERVERS_BIND_METHOD_ANON_USER to the second branch of the if statement as follows, will fix it.

 243     // #2 BIND TO SERVER
 244     $bind_success = FALSE;
 245     if ($ldap_server->bind_method == LDAP_SERVERS_BIND_METHOD_SERVICE_ACCT
 246         ) {
 247       $bind_success = ($ldap_server->bind(NULL, NULL, FALSE) == LDAP_SUCCESS);
 248     }
 249     elseif ($ldap_server->bind_method == LDAP_SERVERS_BIND_METHOD_ANON ||
 250         $ldap_server->bind_method == LDAP_SERVERS_BIND_METHOD_ANON_USER) {
 251       $bind_success = ($ldap_server->bind(NULL, NULL, TRUE) == LDAP_SUCCESS);
 252     }

I guess the patch in #1 was committed? I tried doing what was described in this issue summary, and the message given by Drupal is:

    Password field is required.
    Sorry, unrecognized username or password.

I tested this against 7.x-2.0-beta5+7-dev. Could the maintainer please confirm that this issue has been resolved (without using the module linked to in comment #7 since the project page lists only 1 site using it)?

Is this fixed? The patch seems committed and I can't reproduce this anymore.

I believe it is fixed for 7.x-2.x branch, but not the 7.x-1.x branch.

Even worse, now a new release disguised as "security fix" contains the problem mentioned in #15. Please release a new version of the 7.x-1.x with this fix included.

Trying to discern what the "security fix" actually fixed, or if that was a misnomer?

Agree w/#21. Does 7.x-2.0beta8 even actually have this vulnerability? 7.x-2.0beta9 is completely unusable for us since it does not preserve user roles from LDAP.

edit: I'm asking in a 7.x-1.x thread because this is the only report I found this morning that was at all relevant to what broke on our staging installs when I went to 7.x-2.0beta9 this morning. one for 7.x-2.x is here https://www.drupal.org/node/2779595

The fix in beta9 and 7.x-1.x-beta13 was supposed to fix this. Can someone try and reproduce with those releases. If it doesn't fix it - please post here.

The original bug of this report is fixed in 7.x-1.x-beta13, but not the regression this fix introduced, mentioned in comment #11: 7.x-1.x-beta13 completely breaks LDAP authentication. I had to apply the change mentioned in comment #15 to make ldap authentication work again.

By the way, 7.x-1.x-beta13 also introduced another regression, mentioned in https://www.drupal.org/node/2685607#comment-10999901

cheers, I'll sort a new release in the morning.

Please try the attached against 7.x-1.0-beta13 and report back.

Thanks

@freggy - any feedback? keen to get a new release out with this fixed.

Experienced this regression, patch #26 against 1.0-beta13 works for me. For the record, this is using "Anonymous Bind for search, then Bind with Users Credentials." Which mostly means I'm glad I haven't been vulnerable to a publicly discussed security vulnerability for years.

@larowlan, speaking of releases...just look how harmless https://www.drupal.org/node/2266101#comment-11496735 looks ;)

Production WSOD'd all pages shortly after application of beta13+#26; probably cron ran or cache rebuild was hit. Reverted and was back in business. Will investigate further.

My problem was just #2685607: LDAP_DEREF_NEVER is not (never) defined, exacerbated by a contrib module for error reporting that just blows up completely if there are errors to report.

@mbaynton if we can get #2266101: LDAP User: Allow same e-mail address for more than one user profile (shared e-mail) to RTBC, happy for it to go in the next release. Either way, happy to put out regular releases as things are resolved.

Will aim to get this one out later today to close off the last regression from the security release.

Fixed - thanks, rolling that release