30

Scenario: I have a to-do list that is generated with JavaScript using JSON that was encoded on the server side. I put the todo item id in the HTML id attribute. So the process goes like this:

  1. Server side code creates a todo array.
  2. Serialize array to JSON
  3. Loop through array of JSON objects and render the todo-list.

Now I have to edit a certain to-do item and update it. It is done like this:

  1. I filter my JSON object array by id by comparing the to-do id that came from the HTML id attribute value to get the object.
  2. I use AJAX to pass the object to the INSERT.PHP page.
  3. In the INSERT.PHP page I deserializ the JSON so I can update it in the database.

Problem: Putting the to-do item id in the HTML id attribute will cause a flaw in the system because the user will have the capability to alter the to-do item id using the browsers developer console.

Question: Is there a safe way to do it? Am I just doing it wrong or is this a normal thing to do?

Anders
  • 64,406
  • 24
  • 178
  • 215
user3079341
  • 427
  • 4
  • 5
  • 69
    Everything that can be faked with dev console can be also faked without it, by faking fake requests without even opening the browser. – Tomáš Zato - Reinstate Monica Dec 14 '17 at 19:12
  • 80
    At a past company my boss would have "solved" this issue by disabling right click... – Reinstate Monica Dec 14 '17 at 19:24
  • 10
    https://stackoverflow.com/questions/3483514/why-is-client-side-validation-not-enough – david25272 Dec 14 '17 at 20:36
  • 60
    If the data is sensitive, the problem is not that it's accessible through the developer console but rather that it is at the client at all. – Bergi Dec 14 '17 at 22:17
  • 17
    Why change your code? Just arrest anyone dumb enough to [press F12](https://techcrunch.com/2017/07/25/hungarian-hacker-arrested-for-pressing-f12/). – WernerCD Dec 15 '17 at 13:08
  • 1
    Rule number one regarding user input: You *always* assume it's *all* bad hacky input code. If it's not, your lucky :) – Martijn Dec 15 '17 at 20:24
  • Just had this this afternoon: https://twitter.com/mikeedwardmoras/status/942413008139096064 – e-sushi Dec 17 '17 at 22:09

2 Answers2

160

Most of the details in your question are irrelevant. That the ID is stored in a HTML id attribute, the developer tools, that you are using jQuery... None of that really matters.

The only thing that matters is that you have an endpoint on your server called insert.php. An attacker can send any request they want to that endpoint, regardless of what your client code looks like. Protections against people trying to do things that they are not allowed to do must be at the server, and not the client.

So look at your PHP code. Does it verify that the input is in the expected format? Does it check that the user has the right to edit the particular todo list? If not, fix it. And remember, your validation and authorization checks must be performed on the server to have any security value.

Specifically, if users should only be allowed to edit todos that they own you need to do the following in insert.php:

  • Query the database to get the owner of the todo that is being modified.
  • Get the id for the user making the request.
  • Check that they are the same, and deny if they are not.
Anders
  • 64,406
  • 24
  • 178
  • 215
  • 39
    Make sure you get the logged in ID from a server-side session variable, not something that comes from the client like a form field or cookie. Anything that comes from the client can be forged (session variables depend on a cookie, but it's a long, random string that's presumed to be impossible to guess). – Barmar Dec 14 '17 at 17:32
  • 16
    @user3079341 In general, you should view everything that happens in the browser as a *hint* that you send to the user: "render/do this for best experience". But the user/browser can do *anything* with any of the HTML, CSS, JavaScript that you send them. We trust that *usually* normal users using normal browsers will follow those *suggestions* exactly - but a technical user or a buggy or modified browser can do arbitrary other actions instead. So let the user do whatever on the browser-side, but check for only good/correct data/requests any time the browser sends your server something. – mtraceur Dec 15 '17 at 07:47
  • Also it is helpful to use a tool like fiddler to see exactly what is sent to the server because as already mentioned anything sent to the server cannot be trusted(with the exception of bearer tokens) – Esben Skov Pedersen Dec 15 '17 at 12:54
  • 9
    @Barmar: Also, make sure that the request contains a valid [CSRF token](https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Synchronizer_.28CSRF.29_Tokens), so that another site cannot trick a logged-in user's browser into sending a request to `insert.php` without the user's intent or knowledge. – Ilmari Karonen Dec 15 '17 at 14:17
  • *"Protections against people trying to do things that they are not allowed to do must be at the server, and not the client."* - I agree with first part of this statement, but disagree with second one. Of course protection must be on the server, but it is often convenient to have it also at the client. – Mołot Dec 18 '17 at 11:28
  • 1
    @Mołot Good point. I did not mean to suggest that the client should make it look like everything is possible. But I would not think of it as "protection", but more as UX. – Anders Dec 18 '17 at 11:43
  • @Anders I meant it as lessening the load on the server, so not only UX. – Mołot Dec 18 '17 at 11:48
  • 1
    @Mołot it can only be for UX. You cannot rely on the client side code to lessen the load on the server, unless you mean fewer request by weeding out obviously bad but non-malicious inputs. _Never_ trust your inputs. – Colin Young Dec 18 '17 at 13:04
2

The AJAX related stuff you described doesn't really matter, you should have no trust of the client (browser) and rather, the insert.php script should validate the information.

You need to consider what you are validating - in a case of a todo list, just make sure that the ID they sent, they have the relevant permissions to modify/etc. If anyone does manually send requests to your insert.php script, it should be resilient against whatever it is you need it to be (whether that is, whether it belongs to them/they should have permission to edit it/whether the todo list item is locked/etc).

You should under no circumstances trust the data that the client/browser is sending you and validate everything serverside. You may also want to consider then rather than having an insert.php script which seems to just insert a JSON object, you could create an API for your AJAX requests rather than just inserting JSON objects without validating anything about it.

Jack
  • 21
  • 3