Search code examples
c#asp.net-mvccheckmarkcheckmarx

Unsafe Object Binding in Checkmarx scan


I have tried adding null check and try catch blocks but I am not able to solve this issue.

Error : The DeleteConfirmed at VCSSource/Web/Controllers/EnvController.cs in line 180 may unintentionally allow setting the value of SaveChanges in DeleteConfirmed, in the object VCSSource/Web/Controllers/EnvController.cs at line 180

Code :

[HttpPost, ActionName("Delete")]
[ValidateAntiForgeryToken]
public ActionResult DeleteConfirmed(int id)
 {
     if (id > 0) // fix for CheckMarx : Unsafe Object Binding
        {
           ENV eNV = db.ENVs.Find(id);
           ENV eNV_del = db.ENVs.Remove(eNV);              
            try
              {
                  if (eNV_del != null && eNV_del.ENV_NM.Length > 0) {
                       db.SaveChanges();
                      }                   
                return RedirectToAction("Index");
              }
            catch (DataException ex)
              {
                   throw ex;
              }
            }
     else
       {
          return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
       }
   }

Solution

  • I believe the issue that it is highlighting is that you take an integer ID in as input and directly act upon that ID without any further validation (i.e. anyone can hit this endpoint with that ID and delete that item).

    Part of this issue is that it is then trivial for an attacker to iterate over integers starting from 1 and delete anything in ENV.

    You need to either mark this as 'not exploitable' as you understand how this endpoint is locked down within the application or potentially introduce a secondary method of looking up this object (e.g. a GUID identifier on the table) and have the method take the GUID identifier instead of the integer primary key.