Make WordPress Core

Opened 16 years ago

Closed 14 years ago

#6754 closed defect (bug) (wontfix)

Improve default wp_salt()

Reported by: filosofo's profile filosofo Owned by: ryan's profile 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 16 years ago.
better_wp_salt_prefix2.diff (547 bytes) - added by filosofo 16 years ago.

Download all attachments as: .zip

Change History (13)

#1 @markjaquith
16 years ago

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

#2 @ryan
16 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.

#3 @filosofo
16 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.

#4 @ryan
16 years ago

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

#5 @mdawaffe
16 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.

#6 @Denis-de-Bernardy
15 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

#7 @aviewanew
15 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...

#8 follow-up: @Denis-de-Bernardy
15 years ago

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

#9 in reply to: ↑ 8 @filosofo
15 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.

#10 @Denis-de-Bernardy
15 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.

#11 @ryan
14 years ago

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