Fixed vBulletin 5 Authentication handler seems to be completely broken

[Kirby]

Affected version

2.2.12

Re https://xenforo.com/community/threads/merging-accounts-who-prevails.211508/#post-1608017

I don't have access to vBulletin 5 authentication data, but from looking at the code the handler sees to be completely broken:

protected function getHandler()
{
    return new PasswordHash(\XF::config('passwordIterations'), false);
}

protected function createHash($password)
{
    return md5($password);
}

public function authenticate($userId, $password)
{
    if (!is_string($password) || $password === '' || empty($this->data))
    {
        return false;
    }

    $userHash = $this->createHash($password);
    return password_verify($userHash, $this->data['token']);
}

1. Method getHandler is not being used anywhere (most likely forgotten to be removed when using Core12 as the base?)
2. Function password_verify can't verify plain md5 hashes
3. The data in token isn't a md5 hash, it's Argon2Id or Blowfish
   

   if (preg_match('#^(blowfish:|argon2id:)#iU', $user['scheme']))
   {
       $import->setPasswordData('XF:vBulletin5', [
           'token'  => $user['token']
       ]);
   
       return $import;
   }
   else if ($info = explode(' ', $user['token']))
   {
       $import->setPasswordData('XF:vBulletin', [
           'hash' => $info[0],
           'salt' => $info[1]
       ]);
   
       return $import;
   }

 

[Kirby]

I should not post bug reports late at night when half asleep ...

While it does look strange, mentioned issues 2) and 3) are in fact correct - vBulletin 5 does apply md5 to the password before hashing it (jesus knows why!), so this md5 hash is the correct input for password_verify

The getHandler method still seems useless though, so at least this report isn't entirely wrong

 

[K a M a L]

vBulletin 5 does apply md5 to the password before hashing it (jesus knows why!), so this md5 hash is the correct input for password_verify

it was always the same way since vbulletin 3 .. passwords were converted to md5 hash by Javascript before submitting the login form to avoid being discovered by Man-in-the-middle attack when web was less secure.

 

[Kirby]

it was always the same way since vbulletin 3 .. passwords were converted to md5 hash by Javascript before submitting the login form to avoid being discovered by

Yeah, I know - but It just doesn't seem to make sense to me to use that for new hashes.

vBulletin 3 / 4 databases store md5(md5($password) . $salt) and vBulletin 5 preserves those legacy passwords.
So plain md5($password) is not available (to upgrade existing records, this would only be available from direct user input.

So why they thought it would be a good idea to use such pre-hashing (which might be suspectible to password shucking) is beyond me
(I would have understood if they re-hashed existing passwords though).

 

[XF Bug Bot]

Thank you for reporting this issue, it has now been resolved. We are aiming to include any changes that have been made in a future XF release (2.2.13).

Change log:

Remove dead code from vBulletin 5 authentication handler

There may be a delay before changes are rolled out to the XenForo Community.