Opened 9 months ago
Last modified 2 months ago
#62954 new enhancement
Reassess the best way to store, hash, and check post passwords
| Reported by: |
|
Owned by: | |
|---|---|---|---|
| Milestone: | Awaiting Review | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | Posts, Post Types | Keywords: | 2nd-opinion |
| Focuses: | Cc: |
Description
Previously:
Post passwords are a curiosity. They are stored in plain text in the posts.post_password field (so they can be displayed to an editorial user on the post editing screen). When a user visits a password-protected post and enters a password, the entered value gets hashed with phpass and the hash is stored in the wp-postpass cookie in their browser. Validation happens server-side by validating the user-provided hash against the plain text password in the database.
This is backwards. Storing a password in plain text in the database and then providing its hash via a cookie means that the hash is user-controllable. A malicious actor could use this to exploit a weakness in the hash checking process that they would not otherwise be able to do by providing only the plain text password. There is no known such weakness in the phpass library used in core, but this does prevent us from switching to password_verify() for post passwords where the algorithm, cost, and hash would all be user-controlled.
Given that a post password is actually a shared secret and its hash is not tied to a session or a user, stealing the cookie achieves the same thing as discovering the plain text post password. Hashing the cookie doesn't protect against XSS stealing the cookie for the same reason.
So what is the threat model for the current post password storage, hashing, and checking mechanism? Password re-use is one potential threat, what about others? Let's assess and determine if there is a better mechanism to use for post passwords that doesn't allow the hash to be user-controlled.
Perhaps an access token system can be implemented instead of password hashing. Ideas welcome.
Change History (2)
#1
in reply to:
↑ description
@
2 months ago
#2
@
2 months ago
I've been giving this a second thought, and I think that the performance issues could be solved if, instead of storing user hashes in the posts table as I originally thought, they are stored in the users table. Now wondering if it would be better to store a single record with all the post key pairs, or separate records. Probably separate records with and the meta key being the name and the post like _tokenized_access_token_{post_id} and the value being the hash.
For the BC part, the fact that the passwords are being stored in plain text its obviously an advantage to implement anything because we can receive the password in plain text, check it in plain text without having to deal with nothing in particular during the first check, after that the tokenized hashes will do the thing. Anyway, with a XSS the cookie could be stolen anyway. AFAIK, I'm not too savvy on security topics, but anything-access-token could be stolen and is prone to XSS attacks. I feel that this falls more into Security than Post/Posts Types component, by the way.
A malicious actor could use this to exploit a weakness in the hash checking process that they would not otherwise be able to do by providing only the plain text password. There is no known such weakness in the phpass library used in core, but this does prevent us from switching to password_verify() for post passwords where the algorithm, cost, and hash would all be user-controlled.
This is the only part I'm not strictly sure if its an advantage though (not the password_verifypart but the fact that someone could take or intercept the sent hash and use it for anything in particular). After all the only difference here is that the post_password is constantly being hashed in post_password_required and compared towards the hash of the cookie session for the editors thing that john mentioned. But still I think its more about the fact that phpass is a little outdated vs newer alternatives than the fact that could be broken anytime soon.
Replying to johnbillion:
This makes sense to me. Recently I've been playing around with certain tickets referencing troubles with the password protected posts. Probably because of a local hashing limitation that it's hard to debug given how hard to retrieve is their environment variables.
I'm trying to figure out what would be a good patch for this.
For password generation I assume that
password_hashon the fly is the way to go here.Handling of the password could be in this form (I've picked part of this content, like the randomization, from a plugin snippet that does something similar).
$pw = $_POST['post_tokenized_pass']; $hash = password_hash(the_post_password()); if ($hash && password_verify($pw, $hash)) { $token = bin2hex(random_bytes(24)); setcookie('post_token_' . $post->ID, $token, time() + 30 * DAY_IN_SECONDS, COOKIEPATH, COOKIE_DOMAIN, is_ssl(), true); update_post_meta($post->ID, '_tokenized_access_token', $token); }Which could end up generating a bunch of access tokens in the DB (given the structure of the non-indexed WP post-meta database, could be inefficient, especially if for some reason the post has a massive activity volume, there is no single meta-key that is stored massively for a single post). If we could index the users and posts in a 3-column table, this would be extremely performant, but we don't have this structure in the DB.
So basically, with an access token approach, two potential issues come to my mind:
Any thoughts?