Page MenuHomePhabricator

Autocreation of accounts allows short passwords
Closed, ResolvedPublic

Description

Author: ryan.lane

Description:
In function processLogin in SpecialUserlogin.php, we create accounts
automatically if the external plugin allows it, but we do not check to see if
the password given is valid or not.

This allows users to create accounts with passwords that are shorter than
$wgMinimalPasswordLength. After the user creates the account, the user cannot
log in.

Also, in function addNewAccountInternal, we do not declare the global
$wgMinimalPasswordLength before use.

Below is a patch to fix these two problems.

  • SpecialUserlogin.php.old 2005-11-25 14:49:28.690823592 -0600

+++ SpecialUserlogin.php 2005-11-25 14:57:37.073578128 -0600
@@ -161,11 +161,11 @@

 */
function addNewAccountInternal() {
        global $wgUser, $wgOut;
        global $wgUseLatin1, $wgEnableSorbs, $wgProxyWhitelist;
        global $wgMemc, $wgAccountCreationThrottle, $wgDBname, $wgIP;
  • global $wgAuth;

+ global $wgAuth, $wgMinimalPasswordLength;

// If the user passes an invalid domain, something is fishy
if( !$wgAuth->validDomain( $this->mDomain ) ) {
        $this->mainLoginForm( wfMsg( 'wrongpassword' ) );
        return false;

@@ -288,17 +288,21 @@

if( is_null( $u ) ) {
        $this->mainLoginForm( wfMsg( 'noname' ) );
        return;
}
if ( 0 == $u->getID() ) {
  • global $wgAuth;

+ global $wgAuth, $wgMinimalPasswordLength;

     /**
      * If the external authentication plugin allows it,
      * automatically create a new account for users that
      * are externally defined but have not yet logged in.
      */
     if ( $wgAuth->autoCreate() && $wgAuth->userExists( $u->g
etName() ) ) {

+ if ( !$wgUser->isValidPassword( $this->mPassword

) ) {

+ $this->mainLoginForm( wfMsg( 'passwordto

oshort', $wgMinimalPasswordLength ) );

+ return;
+ }

             if ( $wgAuth->authenticate( $u->getName(), $this
->mPassword ) ) {
                     $u =& $this->initUser( $u );
             } else {
                     $this->mainLoginForm( wfMsg( 'wrongpassw
ord' ) );
                     return;

Version: 1.5.x
Severity: major

Details

Reference
bz4081

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 8:58 PM
bzimport set Reference to bz4081.
bzimport added a subscriber: Unknown Object (MLST).

ryan.lane wrote:

I guess since this is a security problem, allowing people to create accounts who
shouldn't, that I should change this to major.

Is the bug that authentication plugins may try to slip illegal things into the system?

ryan.lane wrote:

Yes. Specifically accounts with passwords that are too short.

If login and creation is partly deferred to an auth system, then that system should actually check these things.

*Both* should check it. Depending on how you are saving passwords it may be impossible for the backend authentication system to check the password. For instance, it is completely valid to save hashes in the LDAP server as SSHA, SHA, MD5, CRYPT, SCRYPT, etc. If all the backend server is getting is a hash, how could it possibly check the length of the password?

Also, if we are enforcing password limits, we need to do so consistently.

Actually, I didn't notice this was 1.5...I don't even think that is supported anymore. Anyway, this is checked on 1.11/1.12, and the password is sent to $wgAuth.

Ah. Looks like it is indeed fixed. I guess I should have checked SVN before reopening this.