Improve security of wp_nonce implementation

[chaoix]

The current wp_nonce implementation is a little out dated and should be improved. While nonces aren't security, a strong nonce implementation can provide some security against form field manipulation.

I have attached a mu-plugin I wrote to test a new nonce algorithm. I will convert it to a patch if there is interest in improving this in core. I have been running this mu-plugin on several high traffic sites I manage with no issues for over 6 months now.

[chaoix]

Secure WP Nonces mu-plugin

[johnbillion]

Thanks for the ticket @chaoix.

Can you provide some information about the algorithm and the changes you've made from core's current implementation please? What aspects make it more secure? Do you have any test coverage? etc.

Thanks

[knutsp]

I have tested the attached plugin for a month on four sites. No issues observed, except that it reports undefined variable $value in line 40.

[johnbillion]

@chaoix Any update on the above? This is a mystery change without more information from you :-)

[chaoix]

I have provided an updated implementation of my changes to the nonce algorithm with more detailed comments.

The benefits of these changes are:

• Uses the sha512/sha256 hashing algorithm for increased nonce length and performance improvements with longer nonce actions. Longer actions prevent brute forcing of the nonce for known action names. MD5 was the previous hashing algorithm used and is not secure enough for what nonces are being used for in WordPress. ​https://en.wikipedia.org/wiki/MD5#Security
• Adds complexity to the nonce hash algorithm to make them more difficult to reverse engineer using rainbow tables.
• Adds a browser id to the nonce action to help prevent known hash reuse.
• Reject nonces from browsers with no or invalid user agent strings. This will prevent lazy bots from submitting requests.

I have had these changes running live on 4 higher traffic sites with no issues. I have not run or made any unit tests for these changes.

[kkmuffme]

I think this is definitely something that could be added easily, however the PR needs a bit of work.

• Why do you hash the user agent, when you then hash everything later on anyway?
• the user agent might not be set if the request comes from PHP CLI, atm this produces a notice in the patch
• redudant code line 26/27?
• always use SHA512, since it's faster
• why truncate to 64 characters?
• hash_hmac seems useless as well as the explode making it unnecssarily complex/slow
• duplicate code with the user agent in 2 functions

[johnbillion]

This is certainly interesting but in order to progress this I'd like to see:

1. A clear explanation of weaknesses in the current approach
2. How the proposed changes address them
3. A patch file or PR

In order to assess an improvement to a security feature such as this we need a clear understanding of the problem. I'm not aware of a weakness in the current nonce system despite its underlying use of md5. The weaknesses of md5 don't affect to a nonce because a nonce isn't secret and collision attacks aren't relevant. A nonce in WordPress is short lived (up to 48 hours) and consists of a 10 character hexadecimal string which means it has over 1 trillion possible values (16¹⁰).

• Is there a way to reduce the size of that search space in order to make a brute force attack viable? Bearing in mind that in order to attack a nonce you need to do so over HTTP, not offline.
• What significance do rainbow tables have? The value used in the hash for a nonce changes over time, according to the user ID, the user's current session token, and the salt that's unique to the site.

I'd be very happy to consider improvements but not without first having a clear understanding of the problem.

[johnbillion]

Closing this off as there's been no clear information provided about the weakness in the current approach. The points in my comment above still stand.

Also a reminder that a nonce should never be used on its own for authentication, it's a CSRF protection that verifies intent and should always be accompanied by a user capability check.

Thanks!