WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 6 years ago

#6754 closed defect (bug) (wontfix)

Improve default wp_salt()

Reported by: filosofo Owned by: ryan
Milestone: Priority: low
Severity: minor Version:
Component: Security Keywords: SECRET_KEY wp_salt security
Focuses: Cc:

Description

As pointed out here, if someone gets the salt from db and SECRET_KEY is default or blank, the password security is no better off than it was in 2.3.3.

My patch adds a md5 hash of the time wp-config.php was last modified and the database password, as a prefix to the secret key. Neither should be available just from obtaining a database dump, and particularly the time wp-config.php was last modified should be difficult to determine, so that should reduce the effectiveness of such an attack as described above.

Attachments (2)

better_wp_salt_prefix.diff (582 bytes) - added by filosofo 7 years ago.
better_wp_salt_prefix2.diff (547 bytes) - added by filosofo 7 years ago.

Download all attachments as: .zip

Change History (13)

comment:1 @markjaquith7 years ago

Clever solution. At first glance this seems like a good bet.

comment:2 @ryan7 years ago

I'd rather not introduce DB_PASSWORD into cookie key creation in any way. Best to avoid any possibility of DB_PASSWORD being brute forced. Can we make do with just mtime and only use it in the recipe only if SECRET_KEY is not defined? An admin might not want to expire cookies for everyone whenever wp-config.php is changed. I certainly wouldn't want that on wordpress.com.

comment:3 @filosofo7 years ago

Okay, how about the second patch? I guess there really isn't a great advantage to including the database password anyways: if you can get the file modified time, you can probably read wp-config.php anyways.

comment:4 @ryan7 years ago

Looks good to me. I'll try it out on my blogs.

comment:5 @mdawaffe7 years ago

Not for or against this patch, but you can sometimes get the mtime of a file you can't read.

stat -f "%m" file

works on files you can't read as long as you can read the directory the file is in. So does ls -l for that matter.

That's probably pretty rare for wp-config.php.

comment:6 @Denis-de-Bernardy6 years ago

  • Component changed from General to Security
  • Owner changed from anonymous to ryan
  • Priority changed from normal to low
  • Severity changed from normal to minor

comment:7 @aviewanew6 years ago

  • Cc aviewanew added

I'm not that familiar with wordpress internals, but if you added the filetime to the hashes, wouldn't it screw you later if you edit wp-config? If you migrate to a new server, or change you db password, or just "touch" it, the time will get updated, and any submitted password won't hash to the same value...

comment:8 follow-up: @Denis-de-Bernardy6 years ago

you'd get logged out indeed. it could be a big deal if you activate WP-Cache or similar plugins. :-)

comment:9 in reply to: ↑ 8 @filosofo6 years ago

Replying to Denis-de-Bernardy:

you'd get logged out indeed. it could be a big deal if you activate WP-Cache or similar plugins. :-)

Just to be clear, I think aviewanew is concerned that he wouldn't be able to log in with his current password, which would not be the case. Changing the salt here would just invalidate your current login session.

Presumably if you're editing your wp-config.php file or moving servers, it won't be too much trouble to clear your cache.

comment:10 @Denis-de-Bernardy6 years ago

right. but on my end I was meaning something like, you click the activate cache button in a caching plugin. it adds a constant to the wp-config.php file -- and boom, you're logged out.

comment:11 @ryan6 years ago

  • Milestone 2.9 deleted
  • Resolution set to wontfix
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.