Make WordPress Core

Opened 10 years ago

Last modified 5 years ago

#27740 new defect (bug)

Passwords consisting of spaces are valid at install time

Reported by: nfreader's profile nfreader Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.8.2
Component: Upgrade/Install Keywords: has-patch needs-refresh
Focuses: Cc:

Description

Steps to reproduce:

  1. Install a fresh copy of Wordpress (after initial database information page)
  2. Fill out the relevant fields (username, email, site title)
  3. On the password fields, press the spacebar once (so your password is one space)
  4. Submit

The install completes and prompts you to log in, using your single-space password

The login page returns an error:
Error: The password field is empty.
..rendering login impossible

I believe this is similar to #24973. Preferably there would be a check during install to see if the password consists of just spaces (or other characters that would be stripped).

Attachments (2)

27740.diff (1.3 KB) - added by izem 10 years ago.
adding poka-yoke to wp-admin/install.php
27740.2.diff (1.4 KB) - added by UmeshSingla 10 years ago.
Cleaned up patch

Download all attachments as: .zip

Change History (12)

#1 @SergeyBiryukov
10 years ago

  • Component changed from General to Upgrade/Install

#2 @izem
10 years ago

[this is my first time trying to contribute to WordPress, hope I'm doing it right]

I've reproduced this on version 4.0-alpha-28611-src

The whitespace-only password is passed as-is from wp-admin/install.php to wp_install function at wp-admin/includes/upgrade.php, there a trim function is used that make it an empty string. If password is empty, wp_generate_password function is called to generate a random password for the user (in according to: "A password will be automatically generated for you if you leave this blank.").
Thing is, user didn't leave the password blank and might expect it to be the whitespace-only password he entered and not a random password that will be emailed to him later.

To avoid this we can add another check at wp-admin/install.php
I've made a patch that check if admin_password isn't empty, but becomes empty if trim() is used on it.

Last edited 10 years ago by izem (previous) (diff)

@izem
10 years ago

adding poka-yoke to wp-admin/install.php

#3 @izem
10 years ago

  • Keywords has-patch added

#4 @yoavf
10 years ago

For reference, this is what happens without the patch if the password fields contains whitespace only:
https://cloudup.com/cc5vEvjL37p

The error message mentions a random password that has been created, but doesn't show it.

Tested 27740.diff - looks good to me.

@UmeshSingla
10 years ago

Cleaned up patch

#5 follow-up: @UmeshSingla
10 years ago

Patch 27740.diff works fine, I've just cleaned it up a bit.

Version 0, edited 10 years ago by UmeshSingla (next)

#6 in reply to: ↑ 5 ; follow-up: @izem
10 years ago

Replying to UmeshSingla:

Patch 27740.diff works fine, I've just cleaned it up a bit.

You didn't cleanup the code, you've changed the functionality - now users can not get an automatically generated password by leaving the password field blank.

#7 in reply to: ↑ 6 ; follow-up: @UmeshSingla
10 years ago

Yes I missed it totally, and I came across this as well http://stackoverflow.com/questions/632167/should-users-be-allowed-to-entered-a-password-with-a-space-at-the-beginning-or-e

So your patch seems to be more appropriate, although the error message could be more formal. Also !empty($admin_password) would return false for a password containing space only. I'm not sure if it will work properly.

Last edited 10 years ago by UmeshSingla (previous) (diff)

#8 in reply to: ↑ 7 @izem
10 years ago

Replying to UmeshSingla:

Yes I missed it totally, and I came across this as well http://stackoverflow.com/questions/632167/should-users-be-allowed-to-entered-a-password-with-a-space-at-the-beginning-or-e

He might have a point there, though the common user is more likely to have the leading/trailing whitespace in its password by mistake. Anyway, its out of this bug's scope (no trimming is done at wp-admin/install.php, and this case deals with whitespace-only passwords).

So your patch seems to be more appropriate, although the error message could be more formal.

English is not my native language, if you have a more-formal/better message please post it.
[The message I've used is: 'Your password has nothing but whitespace. Please try again.']

Also !empty($admin_password) would return false for a password containing space only. I'm not sure if it will work properly.

No, it will return TRUE for the case of a password containing one or more whitespaces.

test.php:
<?php
        $admin_passwords = array('', ' ');
        foreach($admin_passwords as $admin_password) {
                print "'{$admin_password}': " . (!empty($admin_password) ? 'admin_password not empty' : 'admin_password is empty')
 . "\n";
        }
?>

[root@wp_test tmp]# php ./test.php
'': admin_password is empty
' ': admin_password not empty

I've tested the install form with my patch before attaching it here, it worked as expected in both cases (empty and nonempty). You are welcome to test is as well.

#9 @chriscct7
8 years ago

  • Keywords needs-refresh added

#10 @SergeyBiryukov
5 years ago

#45100 was marked as a duplicate.

Note: See TracTickets for help on using tickets.