0

Please refer to my comments on Greg's answer for more details in relation to his answer which are also relevant to this.

I'm routing all my AJAX requests through a Controller, this is a class that simply localizes an endpoint and calls it further, as such, it's some sort of a router, it does some checks first:

  1. This Controller, on initialization, creates a nonce for the user in question. This nonce is global. Endpoints themselves don't have a nonce themselves and they are not registered as AJAX endpoints (as such, an attacker cannot simply bypass having to go through the Controller, as such, a nonce check is only done once.
  2. Runs an access callback function such as can_user( 'administrator' ) every time it's called.

As such, my Controller has two layers of security: one to check whether or not the request was made on behalf of the user who the Controller gave that nonce to and another check to see if whoever is calling and endpoint has rights to (since all endpoints are open to everyone by default).

My intention here was to make the endpoints creation seamless and not have a lot of boilerplate if unnecessary and as such, I didn't want those who extend the system to have to create a nonce for each action but rather rely on an access callback to see if an action should happen.

My question is, given that wp_create_nonce takes an argument of $action, that is to say, it tries to contextualize a nonce to a specific action (which my Controller isn't. The Controller just simply calls an action and, supposedly, if that action wasn't called by same user, it bails), is this enough?


A side note.

I have tested the classic: access a link/complete a form to my website that would, in theory, do an action that an admin wouldn't want to, e.g. I tried to trick myself. The error given is, as expected "403: Forbidden" by means of the nonce being invalid.

My question is, then: why did the WP Core team have a need to try to bind a nonce to an action? Just for the sake of clarity?

If that is the case, it's definitely because the WP Core didn't write AJAX endpoints and the system to be put under a router-like-fancy-pants structure and so, they tried to single out endpoints with context so that they can be made sense of. In a system like mine, it's already made sense of by extension of me having a Container, Provider and helpful functionality to tell information about Endpoint objects.

3

With the user-name-based nonces, i think you are trying to protect yourself from CSRF attacks, this should work to some extent (hard to tell without the full source code), that being said the nonces should be assigned to a specific action (not only an action), so, for example, you do not assign it to a "delete item" action but rather to "delete item with ID=X" action.

Here is why you should do it like that.

Lets assume you have 2 users one with login "john" and the other with login "hacker".

If you generate the nonce per user only then for each user you could generate a nonce like "{$user_name}-delete-item". Any logged-in user would have this nonce generated and by looking in the source code they could find out what their nonce code is.

Now imagine the "hacker" user has a delete link which looks like this https://example.com/?delete-item=1000&nonce=qwertyuiop where (qwertyuiop is his generated nonce code).

The "hacker" user could manipulate the link and replace 1000 with for example 2000 and open the link in the browser. The nonce would validate and the "hacker" would be able to delete the item with id 2000 even though the "john" user owns it.

To improve the security you should generate a nonce name like "my-delete-item-{$id}" (in this case "my-delete-item-1000"), then nonce will be generated for a specific action, and if the "hacker" will change the ID from 1000 to 2000 the nonce will not validate as he does not have a nonce generated for "my-delete-item-2000" only for "my-delete-item-1000".

Of course, i assume you have some additional code for checking if the user owns the 2000 item so you might get away with using nonces like that in this case, my point is just that if the nonce is not generated for a specific action it might create a false sense of security.

One final note, even with generating the nonces per specific action if possible you should have additional code for checking if the user can perform the given action. As pointed out in the nonces documentation https://codex.wordpress.org/WordPress_Nonces

Nonces should never be relied on for authentication or authorization, access control. Protect your functions using current_user_can(), always assume Nonces can be compromised.

5
  • Very insightful and this debunks my fear. I am offsetting the checks to my "access callback function". I'll try to explain as best as I can. When an Endpoint gets called from my Controller, the Controller passes the data it was sent from the front-end to the Endpoint's function isAllowed( $data ). This function isAllowed's sole purpose is that to check [anything that the creator of the endpoint deems relevant for security] - that contains ownership and so on. The idea with this function is that the creator must decide how to grant access.
    – Daniel M
    Jan 29 '20 at 17:26
  • So, as per your case, the Controller will pass the Endpoint through isAllowed, the data ['id' => 1000], it is then isAllowed's job to decide whether or NOT whoever is making the current request should be allowed or not to proceed and call the actual "doing things" function of the Endpoint, that is doEndpoint where the post actually gets deleted.
    – Daniel M
    Jan 29 '20 at 17:28
  • Why all this? Why not do as you say, even if it's a good idea? My goal here is to offset as much work from these who want to create/hook to endpoints. When you create a nonce, you have to also localize it. Although an Endpoint can only ever have a nonce to check against, when people are creating their markup, forms, JS, they'd have to be aware of the Endpoint's nonce name. With my way, they only simply need to pass myPackage.security in the security field of the POST request and it's taken care of.
    – Daniel M
    Jan 29 '20 at 17:31
  • That is all to say that, when I call an endpoint, I wanna tell myself Dude, I don't care about reading up on what each Endpoint I need has for a nonce name just so then I can pass it, just let me do my thing. and not worry. The back-end should decide whether or not it will grant me passage or not based on granular checks. So, the heavy-lifting is done by the creators of Endpoint objects, these who use them just simply call them.
    – Daniel M
    Jan 29 '20 at 17:36
  • ...which makes me think...why do I even need nonces...at all, then? I guess as a first-defense against developers who don't care enough to do granular checks.
    – Daniel M
    Jan 29 '20 at 17:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.