Fixed vBulletin 5 Authentication handler seems to be completely broken

Kirby

Well-known member
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:

PHP:
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
    PHP:
    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;
    }
 
Last edited:
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 ;)
 
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.
 
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).
 
Last edited:
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.
 
Back
Top Bottom