[FIXED][3] passwords - user may set a "blank" password!

[Turtullo]

i hope this wasn't discussed earlier.

so here's the problem:

user goes to his CP and wants to change his password. accidentaly hits "enter" after filling in the first input-field of the passchange-form (his present password). there is no warning that the new password is too short and vb sets a blank password! (user may now log in with only his username!)

i have noticed it on ver. 2.2.6, so maybe it was already fixed, if not, it should be fixed IMHO.

cheers

[Chen]

Doesn't look like it was addressed in 2.2.9 - moved. :)

[XiXora]

2.2.10 here we come...

[Xenon]

Originally posted by XiXora
2.2.10 here we come... hmm, not a serious bug...

if a user wants to have just his username, nothing more to login, it's his own fault ;)

just remember mods they shouldn't do this ;)

[Chris M]

Originally posted by Xenon
hmm, not a serious bug...

if a user wants to have just his username, nothing more to login, it's his own fault ;)

just remember mods they shouldn't do this ;) Try installing Logician's "Advanced Password Rules" hack from vBulletin.org - It doesn't appear to set the password as blank with it installed:)

Satan

[CeleronXT]

Originally posted by hellsatan
Try installing Logician's "Advanced Password Rules" hack from vBulletin.org - It doesn't appear to set the password as blank with it installed:)

Satan Is there any official fix for this yet? I am not going to upgrade to 2.2.10, I am just applying the various fixes (such as the one from the above thread).

[Tommy Boy]

Oh what the heck, here we go. :D

Open member.php, and look for:
// validate old passwordBefore it, add:

// Password minimum length check (FIX: http://www.vbulletin.com/forum/showthread.php?threadid=59983)
$newpassword = trim ($newpassword);
$newpasswordconfirm = trim ($newpasswordconfirm);
if (strlen ($newpassword) < 4)
eval("standarderror(\"".gettemplate("register_error_passwordtooshort")."\");");

Open register.php, and look for:
// check max imagesBefore it, add:

// Password minimum length check (FIX: http://www.vbulletin.com/forum/showthread.php?threadid=59983)
if (strlen ($password) < 4)
eval("standarderror(\"".gettemplate("register_error_passwordtooshort")."\");");

Add a new template, called register_error_passwordtooshort:
Your password should be at least 4 characters long. Please correct and try again.
Important: This is not meant to be a hack, but rather a fix to this specific problem. Sure, it provides an added value of enforcing a minimum password length, but this length is constant in the code, and cannot be changed from the Admin panel. Currently, the limit is set to 4 characters. If you want to change it, replace all 4's in the codes above to your desired length.

[Paul]

You'll need to add:


exit;
}

after both instances of:


if (strlen ($password) < 4)
eval("standarderror(\"".gettemplate("register_error_passwordtooshort")."\");");

in TommyBoy's fix.

For extra fun and to liven things up, change "4" to "300" to force 300-character passwords and mess with your users' heads.

:D

[Paul]

Originally posted by LoveShack
You'll need to add:


exit;
}

after both instances of:


if (strlen ($password) < 4)
eval("standarderror(\"".gettemplate("register_error_passwordtooshort")."\");");

in TommyBoy's fix.

I'd also recommend putting the password length check ABOVE the following code in register.php instead of the suggested location supplied by TommyBoy:


if ($password!=$passwordconfirm) {
eval("standarderror(\"".gettemplate("error_passwordmismatch")."\");");
exit;
}

[Chen]

Originally posted by LoveShack
You'll need to add:


exit;
}

after both instances of:


if (strlen ($password) < 4)
eval("standarderror(\"".gettemplate("register_error_passwordtooshort")."\");");

in TommyBoy's fix.

For extra fun and to liven things up, change "4" to "300" to force 300-character passwords and mess with your users' heads.

:D No you don't, standarderror() dies on its own.

[nuno]

Originally posted by Xenon
hmm, not a serious bug...

:confused: :eek:

[Xenon]

Originally posted by nuno
:confused: :eek: as chen said, standarderror has an exit; in it ;)

@nuno: ?
If a user sets up an empty pw, it's his faul, not a bug to me.....

for fixing in vb2.2.10 i'd say an settingoption should be created for this

[nuno]

Originally posted by Xenon
as chen said, standarderror has an exit; in it ;)

@nuno: ?
If a user sets up an empty pw, it's his faul, not a bug to me.....


Originally posted by Turtullo
i hope this wasn't discussed earlier.

so here's the problem:

user goes to his CP and wants to change his password. accidentaly hits "enter" after filling in the first input-field of the passchange-form (his present password).
there is no warning that the new password is too short and vb sets a blank password! (user may now log in with only his username!)

cheers

BUG :o

[Paul]

Originally posted by Xenon
as chen said, standarderror has an exit; in it ;)

@nuno: ?
If a user sets up an empty pw, it's his faul, not a bug to me.....

for fixing in vb2.2.10 i'd say an settingoption should be created for this Oops. I didn't notice the lack of a left brace in TommyBoy's fix. His fix will work as is.

While standarderror() will exit automatically, vBulletin's coding style always puts an exit; after it. If you do add the closing brace, be sure to put one after the condition in the if statements as well--I didn't notice the lack of one when replying to this message before. :)

I'm a stickler for nice uniform code. The change from 4 to 300 is still recommended for a laugh. :D

Thanks for slapping some sense into me.

[Paul]

I agree that this would probably be best applied as an option in the admin cp. In that case <= $passwordlength should be used.

For now, the supplied fix by Tommy Boy is good enough to prevent problems.

[nuno]

Originally posted by LoveShack
I'm a stickler for nice uniform code. The change from 4 to 300 is still recommended for a laugh. :D


lol :D

[Xenon]

as i remember, in older versions the exit wasn't included into standarderror, but in newer ones it is, they just didn't change all the appearence of this ;

[Tommy Boy]

Originally posted by LoveShack
You'll need to add... after both instances of... in TommyBoy's fix. I'd also recommend putting the password length check ABOVE the following code in register.php instead of the suggested location supplied by TommyBoy.
I suggest you first install my hacks/fixes before detemining they don't work. I have too many hours of vb coding in order to post things that don't work.

Just wanted to make that point clear. :D

[Paul]

Originally posted by Tommy Boy
I suggest you first install my hacks/fixes before detemining they don't work. I have too many hours of vb coding in order to post things that don't work.

Just wanted to make that point clear. :D Thank you for the suggestion, Tommy Boy.

I'll make a personal note to myself to be extra vigilent when installing any hacks or fixes suggested by you.

If and when any problems with any hacks or fixes you release arise, I'll make an extra effort to point them out to you. :D

Best wishes,
Paul

[Chris M]

Im afraid I would have to agree with Xenon on that issue of it being the user's fault, even though I personally prefer to have Logician's Advanced Password Hack, so it forces everyone to have a password...

If the user doesnt enter a password, or accidentally presses Enter, then it is their fault, not mine...Perhaps if they are a Moderator or Super Moderator, they should be warned...That could get Security Risky if it happens...

Satan

[Paul]

Originally posted by hellsatan
If the user doesnt enter a password, or accidentally presses Enter, then it is their fault, not mine...Perhaps if they are a Moderator or Super Moderator, they should be warned...That could get Security Risky if it happens...

This clearly will not work in certain situations where vBulletin is implemented. Many sites take user security very seriously. There should be an option in the Admin CP to allow the administrator to define the minimum password length. Don't want to require a password? Set the value to "0". Run a site that requires a high level of password security? Set the value to 7 or 8.

Best wishes,
Paul

[Chris M]

Originally posted by LoveShack
This clearly will not work in certain situations where vBulletin is implemented. Many sites take user security very seriously. There should be an option in the Admin CP to allow the administrator to define the minimum password length. Don't want to require a password? Set the value to "0". Run a site that requires a high level of password security? Set the value to 7 or 8.

Best wishes,
Paul Well Why not just Install Logician's Advanced Password Rules as a Standard vBulletin Function? It does exactly what you said and more...

If you are still that worried about it, install the bug fix in this thread...

If you are security conscious, there are 2 solutions...

I fail to see the problem...

Satan

[Paul]

I disagree completely. This isn't the end-user's fault at all.

The "problem" is that I don't believe it was intended behavior to allow blank passwords in a default installation of vBulletin. If it was, I believe a serious flaw was made in vBulletin's documentation. I'm not suggesting that advanced password policies be made a standard feature to vBulletin 2.x--certainly Logician has provided an excellent hack that takes care of that and I'm sure we can expect to see similar functionality in vB3.

The ability for users to have blank passwords in 2.x is completely undocumented. I'm sure that users on the many vBulletin installations out there have a blank password and I sincerely doubt that most administrators were even aware of this possibility.

For example, I don't think it would be wise for Jelsoft to want users on the vBulletin.com support forums to have blank passwords. The same goes for other implementations of this software where access is somehow limited. Sure--it doesn't really matter for a forums site that discusses book reviews or gaming, but for licensed product support or peer support discussion groups, having such a disregard for basic authentication security probably isn't acceptable. It would be a completely different story if there were a little note in the Admin manual stating something along the lines of "Passwords may be anywhere from 0 characters to ... For those that may want a minimum password length, set such and such a value in the Admin CP or make this edit to your code."

At the very least, notice should be made to license holders who may not know about this limitation. I know I certainly never did until I saw this thread--had I known, it would have been one of the first hacks we would have made to the software. It probably would have influenced our buying decision. I doubt that all license holders visit this forum on a regular basis.

It is significant enough to many vBulletin installations that it requires either documentation clarifying that no password is required or the addition of a simple switch variable that would allow the administrator to set whether or not passwords can be blank or not.

In that sense, I believe it is a "bug." Whether or not it gets "fixed" in future vBulletin 2.x releases matters not to me. I've already made the changes that apply to our installation. I think the responsible thing for Jelsoft to do here is make users aware of what could very well be a potential security issue (perhaps in the next version notification mailing) and provide a solution that will take into account the possibility that some users will have a blank password and a means to set a longer password if wanted during the upgrade process--for example, when running upgrade23.php (or whatever we're up to) have it prompt the administrator with a list of users that have a blank password set and let them choose what to do from there: Perhaps mail them a random new password, or let them retain the blank password if they choose to disregard this completely, or allow users with blank passwords to continue to use them, but should they try to change the password, make sure it's at least so many characters long.

It would be trivial to add an admin definable check to the software. It all depends on Jelsoft's willingness to take what could potentially be bad press. They may choose to blow this off and recommend that concerned users upgrade to vB3 when it becomes available or say that their product wasn't intented to be used in security conscious situations. Perhaps this is a feature, in which case, this just amounts to a poorly implemented and designed area of the product.

Ideally, the password length check should probably be made on a user group basis.

I think I've said enough about this issue. My concern is with other vBulletin license holders. As far as I see it, the problem is solved on my end.

</rant>

[Chris M]

Originally posted by LoveShack
I disagree completely. This isn't the end-user's fault at all.

The "problem" is that I don't believe it was intended behavior to allow blank passwords in a default installation of vBulletin. If it was, I believe a serious flaw was made in vBulletin's documentation. I'm not suggesting that advanced password policies be made a standard feature to vBulletin 2.x--certainly Logician has provided an excellent hack that takes care of that and I'm sure we can expect to see similar functionality in vB3.

The ability for users to have blank passwords in 2.x is completely undocumented. I'm sure that users on the many vBulletin installations out there have a blank password and I sincerely doubt that most administrators were even aware of this possibility.

For example, I don't think it would be wise for Jelsoft to want users on the vBulletin.com support forums to have blank passwords. The same goes for other implementations of this software where access is somehow limited. Sure--it doesn't really matter for a forums site that discusses book reviews or gaming, but for licensed product support or peer support discussion groups, having such a disregard for basic authentication security probably isn't acceptable. It would be a completely different story if there were a little note in the Admin manual stating something along the lines of "Passwords may be anywhere from 0 characters to ... For those that may want a minimum password length, set such and such a value in the Admin CP or make this edit to your code."

At the very least, notice should be made to license holders who may not know about this limitation. I know I certainly never did until I saw this thread--had I known, it would have been one of the first hacks we would have made to the software. It probably would have influenced our buying decision. I doubt that all license holders visit this forum on a regular basis.

It is significant enough to many vBulletin installations that it requires either documentation clarifying that no password is required or the addition of a simple switch variable that would allow the administrator to set whether or not passwords can be blank or not.

In that sense, I believe it is a "bug." Whether or not it gets "fixed" in future vBulletin 2.x releases matters not to me. I've already made the changes that apply to our installation. I think the responsible thing for Jelsoft to do here is make users aware of what could very well be a potential security issue (perhaps in the next version notification mailing) and provide a solution that will take into account the possibility that some users will have a blank password and a means to set a longer password if wanted during the upgrade process--for example, when running upgrade23.php (or whatever we're up to) have it prompt the administrator with a list of users that have a blank password set and let them choose what to do from there: Perhaps mail them a random new password, or let them retain the blank password if they choose to disregard this completely, or allow users with blank passwords to continue to use them, but should they try to change the password, make sure it's at least so many characters long.

It would be trivial to add an admin definable check to the software. It all depends on Jelsoft's willingness to take what could potentially be bad press. They may choose to blow this off and recommend that concerned users upgrade to vB3 when it becomes available or say that their product wasn't intented to be used in security conscious situations. Perhaps this is a feature, in which case, this just amounts to a poorly implemented and designed area of the product.

Ideally, the password length check should probably be made on a user group basis.

I think I've said enough about this issue. My concern is with other vBulletin license holders. As far as I see it, the problem is solved on my end.

</rant> Lol...

Usergroup basis is definately a good idea for a Permanent Solution:)

Personally, I think that Jelsoft should consider the implentation of Logician's Hack, OR making some kind of system that requires Administrators, Moderators and Super Moderators to have a password;)

I still don't consider this a bug though:)

Satan

[Paul]

This was sort of addressed in 2.3.0, however I feel it was not adequately fixed.

In 2.3.0, if a user forgets to submit a new password, vBulletin will display error_wrongpassword. This is misleading to the user. They should be informed that they forgot to enter a new password, not that their old password is incorrect.

Secondly, the input for $newpassword is not being trim()'ed for any spaces that may have been entered before or after the password. I think it's safe to say that in most cases, spaces prefixing or trailing the entered password are accidentally entered and not intended to be a part of the user's password. This probably classifies as a new bug, but was originally suggested as part of the fix in this thread to the original report.

Incidentally, $password is trim()'ed upon new user registration. $newpassword should follow the same rule.

Best wishes,
Paul