Possible bug/security issue

[Gokkesokken]

found this in core/includes/functions_login.php

if ($postvars['securitytoken'] = 'guest')
{
$vbulletin->userinfo['securitytoken_raw'] = sha1($vbulletin->userinfo['userid'] . sha1($vbulletin->userinfo['secret']) . sha1(vB_Request_Web::$COOKIE_SALT));
$vbulletin->userinfo['securitytoken'] = TIMENOW . '-' . sha1(TIMENOW . $vbulletin->userinfo['securitytoken_raw']);
$postvars['securitytoken'] = $vbulletin->userinfo['securitytoken'];
$vbulletin->GPC['postvars'] = sign_client_string(json_encode($postvars));
}

I am no php expert but i don't think '=' is a comparison operator, at least not according to php.net: http://php.net/manual/en/language.operators.comparison.php

[ozzy47]

No it is not a comparison operator, it is a string operator, http://php.net/manual/en/language.operators.string.php

Not a bug or security issue.

[Gokkesokken]

No it is not a comparison operator, it is a string operator, http://php.net/manual/en/language.operators.string.php

Not a bug or security issue.

I understand that this operator assigns $postvars['securitytoken'] with 'guest', but is this intended? And if it is, what does the if do there? please forgive my curiosity.

[ozzy47]

No it does not assign, $postvars['securitytoken'] with 'guest'

What it is is part of the login redirect, saying if the user logging in is a guest, to do this.

[Dave]

I think he means that

if($postvars['securitytoken'] = 'guest')

Will assign guest to the $postvars['securitytoken'] variable.
However, it should be this:

if($postvars['securitytoken'] == 'guest')

I don't know the rest of the code so this might be intended by whoever wrote the script.

[Zachery]

I believe he is correct, it looks like a typo from the vb4 version of the same code.

[tbworld]

This was fixed in vBulletin v4.1.11. :)

[Gokkesokken]

This was fixed in vBulletin v4.1.11. :)

Thanks for letting me know but this is from vb5.


/*================================================= =====================*\
|| ################################################## ################## ||
|| # vBulletin 5.1.2 Patch Level 3 - Licence Number ##########
|| # ---------------------------------------------------------------- # ||
|| # Copyright ?2000-2014 vBulletin Solutions Inc. All Rights Reserved. ||
|| # This file may not be redistributed in whole or significant part. # ||
|| # ---------------- VBULLETIN IS NOT FREE SOFTWARE ---------------- # ||
|| # http://www.vbulletin.com | http://www.vbulletin.com/license.html # ||
|| ################################################## ################## ||
\*================================================ ======================*/


I could upload the file as proof, if it doesn't get me into trouble for copyright violations.

[tbworld]

Thanks for letting me know but this is from vb5.

You know I thought I checked the forum category, right before I posted. Oh well, I am loosing it. :) I guess vb goofed then, and good fine. :)
VB5 was ported from VB4 before they fixed it in VB4, so it makes sense. Make sure you post it to JIRA, or if you do not want to bother let me know and I will handle it.

[Gokkesokken]

You know I thought I checked the forum category, right before I posted. Oh well, I am loosing it. :) I guess vb goofed then, and good fine. :)
VB5 was ported from VB4 before they fixed it in VB4, so it makes sense. Make sure you post it to JIRA, or if you do not want to bother let me know and I will handle it.


Thanks for letting me know, I think your explanation is very plausible and also the most likely given the circumstances. The reason I posted this here first was because I didn't want to submit an inaccurate bug report.

[tbworld]

I have seen a few minor errors like this, yours is the first important one I have seen. Now that more developers are starting to peruse the code, these errors should get weeded out.

[Zachery]

I've reported it as a bug. The code would always run as true.
http://tracker.vbulletin.com/browse/VBV-13404

[ozzy47]

Hmmm, I stand corrected, nice catch on that. :)