0

I'm working on a C# MVC application where users submit applications to administrative users for review. Applications can be approved or denied by administrative users. My home page for administrators renders a list of submitted applications, and clicking on each application opens a new page where applications are processed.

My concern is simple: since the "Id" attribute for each application is a hidden html element on the admin "Process Application" form, it is possible that a user could modify the application ID and submit the form (in turn approving/denying the inappropriate application). I can get around this by using a Session object for "AppId" and verifying the AppId posted by the form is the same as the session AppId.

However (and this is the real problem), if I set Session["AppId"] = applicationId that session object can easily be overridden if the user was to let’s say attempt to process another application before submitting the first. Perhaps the admin user fancies their self a multi-tasker and opens two "Process Application" windows. Essentially, the first Session["AppId"] will be overridden by the second. This causes a problem on postback because now I can't validate anything based on Session.

While writing this, I realize I could add controls to prevent the user from processing more than one application at the same time. Is there an alternative approach though? Also worth noting, only admin users would have the ability to forge an application ID, which is unlikely because the Web-App is meant to help the admin users. Really I’m just looking for a best practice for these scenarios, as opposed to fearing someone will actually forge elements on my form.

Is my best approach actually storing an AppId in session, and preventing admins from processing more than one App at a time (so that the session object isn’t overridden)? It would seem so, but I’d love advice from the community.

PS: I realize this issue is similar to Secure way to stop users from forging forms. However, I think the biggest difference is I’m currently allowing users to process more than one application at a time, which prevents me from using a single session object for “AppId”.

Community
  • 1
  • 1
  • Don't pass secrets to users? Why not pass the user a unique identifier for their session in the URL or as a cookie, and keep all the data safely on the server? This way they have nothing they can edit/corrupt/hack. – whoisj Aug 06 '15 at 18:54

2 Answers2

2

I'd approach this from the "sanity & security" point of view. Users should only be able to change what they are supposed to change, data should [also] be validated on server side, and then you can disregard all forgery :-)

Miloslav Raus
  • 747
  • 6
  • 9
  • I agree. I've decided to check my session object on page load to ensure an AppId isn't already in session. If there is an AppId in session, I'll redirect the user to process that application with an appropriate warning message. Basically, only one application can be processed at a time to prevent the forgery of form elements. –  Aug 06 '15 at 18:25
  • Unnecessary restriction and complication. Check whether they are supposed to edit the ID at all, validate the input and then allow it. No need to cache anything, and if the users can only forge edits they would be able to do by using UI, why would they forge anything ? And why should you care whether it was forged or not, if they have the right to edit it, and the data are correct ? – Miloslav Raus Aug 06 '15 at 18:38
  • But the user shouldn't be able to edit an App ID through the UI. I just need it persisted on the form so I know which app to edit on postback. Using session seems to alleviate this. –  Aug 06 '15 at 18:40
  • "And why should you care whether it was forged or not, if they have the right to edit it, and the data are correct ?" The user has the right to edit everything except the application ID, because editing that would cause the edits to affect a different application. –  Aug 06 '15 at 18:43
  • I assumed the appid is a primary key, so if they would forge it, they'd be editing another record. If it's not primary key, it's not necessary to cache it ... – Miloslav Raus Aug 06 '15 at 18:44
  • It is indeed a primary key, that's why I'm afraid of someone editing it through the UI, because I can't afford to have them edit the wrong record maliciously. –  Aug 06 '15 at 18:46
  • They cannot "edit it", if they forge it, they'll (stupidly) refer to another record. So you just need to validate whether they are allowed to edit that record. The worst that could happen is that some bozo will overwrite some record he would be able to change anyhow, but he won't be able to change the ID of existing record. – Miloslav Raus Aug 06 '15 at 18:52
  • yes, that is a good point. I think you summarized it well. If the user edits the wrong record intentionally, they are screwing up data they could have likely edited directly anyways. –  Aug 06 '15 at 18:56
0

The best approach I’ve found is to check the AppId session object on page load. If it exists, then the user did not finish processing the original application (that scenario can be handled in various ways. I'll let you decide what is best, but you could probably redirect the user back to process the original application with an appropriate warning message explaining what happened). This is the only way I can think of to prevent the forgery of an AppId on the form using a single session object.

h0r53
  • 3,034
  • 2
  • 16
  • 25
  • After further consideration, I think @Miloslav Raus had a good point. My concern is an admin user forging data maliciously, but the admin has the ability to directly edit any records that could potentially be forged. Someone would just have to want to screw up data just for the sake of messing things up for this to become a concern in my project. –  Aug 06 '15 at 18:58
  • I agree that his approach is probably more appropriate. I was just trying to answer the question "how do I prevent users from forging form data" with the simple solution: store the ID for the data you are referencing in session and only let users edit a single form at a time. – h0r53 Aug 06 '15 at 19:01
  • Session, DB, Key-Value store (ie. server-side). Or signing / encrypting the data you want to protect on client side. But there is no protection against changes the user is allowed to do normally. – Miloslav Raus Aug 06 '15 at 19:06