Reputation: 145
I'm working on solving some application vulnerabilities. I have an edit page with a url http://localhost:12997/Manning_HQ/Edit/1274
the problem was users were able to change the the id and be able to access other requests that were not supposed to access.
I searched the URLEncoding
but found that it only ensures that all browsers will correctly transmit text in URL strings. My question is there a way to prevent this?
My edit function:
// GET: Manning_HQ/Edit/5
[CustomAuthorize(Roles = AccessRoleHelper.Add_Manning_Plan_HQ)]
public ActionResult Edit(int? id)
{
if (id == null)
{
return new HttpStatusCodeResult(HttpStatusCode.BadRequest);
}
var test1 = (from c in db.TBL_Manning_HQ
join e in db.TBL_User_Dep_Access
on c.Inserted_By equals e.UserID
join t in db.TBL_Department
on c.Department_ID equals t.Department_ID
join p in db.TBL_Location
on c.Location_ID equals p.Location_ID
join n in db.TBL_Titles_HQ
on c.TitleHQ_ID equals n.TitleHQ_ID
where (e.Dep_ID == c.Department_ID)
where c.Manning_HQ_ID == id
select new Manning_HQ_VM
{
Manning_HQ_ID = c.Manning_HQ_ID,
}).ToList();
TBL_Manning_HQ tBL_Manning_HQ = db.TBL_Manning_HQ.Find(id);
int userid = Convert.ToInt32(Request.ServerVariables["LOGON_USER"].ToString().Split('\\')[1].ToString().Remove(0, 2));
if (tBL_Manning_HQ == null)
{
return HttpNotFound();
}
var departments = (from d in db.TBL_Department
join ud in db.TBL_User_Dep_Access
on d.Department_ID equals ud.Dep_ID
join m in db.TBL_Manning_HQ
on d.Department_ID equals m.Department_ID
where ud.UserID == userid && ud.IsActive == true
where m.Manning_HQ_ID == id
select new KeyValuePairsViewModel
{
Id = d.Department_ID,
Value = d.Department_Name
}).ToList();
ViewBag.Department_ID = new SelectList(departments, "Id", "Value");
ViewBag.Location_ID = new SelectList(db.TBL_Location, "Location_ID", "Location_Name", tBL_Manning_HQ.Location_ID);
ViewBag.TitleHQ_ID = new SelectList(db.TBL_Titles_HQ, "TitleHQ_ID", "Title_Name", tBL_Manning_HQ.TitleHQ_ID);
if (test1.Count != 0)
{
return View(tBL_Manning_HQ);
}
else
{
return View();
}
}
[HttpPost]
[CustomAuthorize(Roles = AccessRoleHelper.Add_Manning_Plan_HQ)]
[ValidateAntiForgeryToken]
public ActionResult Edit(TBL_Manning_HQ _HQ)
{
var chk_ID = db.TBL_Manning_HQ.Any(x => x.Manning_HQ_ID == _HQ.Manning_HQ_ID);
var Chk_Dep = db.TBL_Manning_HQ.Where(x=>x.Manning_HQ_ID == _HQ.Manning_HQ_ID).Any(x => x.Department_ID == _HQ.Department_ID);
if (chk_ID && Chk_Dep)
{
_HQ.Update_By = Convert.ToInt32(Request.ServerVariables["LOGON_USER"].ToString().Split('\\')[1].ToString().Remove(0, 2));
_HQ.Update_In = DateTime.Now;
db.SaveChanges();
TempData["success"] = "Data updated successfully !";
}
return RedirectToAction("Index");
}
Custom Authorize:
public class CustomAuthorize : AuthorizeAttribute
{
Staff_RequisitionEntities_1 db = new Staff_RequisitionEntities_1();
public override void OnAuthorization(AuthorizationContext filterContext)
{
if (filterContext == null)
{
throw new ArgumentNullException("filterContext");
}
if (filterContext.HttpContext.User == null)
{
throw new ArgumentNullException("filterContext.HttpContext.User");
}
string username = filterContext.HttpContext.User.Identity.Name.ClientName();
int xceedId = Convert.ToInt32(username.Substring(2));
var user = db.TBL_UserPermissions.FirstOrDefault(us => us.UserID== xceedId && us.IsActive == true);
{
filterContext.HttpContext.User = new CustomPrincipal((WindowsIdentity)filterContext.HttpContext.User.Identity,user, true , Convert.ToString(xceedId));
base.OnAuthorization(filterContext);
}
}
}
Access role helper:
public class AccessRoleHelper
{
public const string Manning_Plan = "Add Manning Plan";
public const string View_Manning_Plan = "View Manning Plan";
public const string Add_New_Request = "Add New Request";
public const string View_Requestes = "View Requestes";
public const string Add_Fulfillment_Rate = "Add Fulfillment Rate";
public const string View_Fulfillment_Rate = "View Fulfillment Rate";
public const string Add_Manning_Plan_HQ = "Add Manning Plan HQ";
public const string View_Manning_Plan_HQ = "View Manning Plan HQ";
public const string View_Manning_Plan_HQ_CombCEO = "View Manning Plan HQ Manager";
public const string View_Manning_Plan_CEO = "View Manning Plan CEO";
public const string Add_Issue_staff_requisition = "Add Issue staff requisition";
public const string Training_Action = "Training Action";
public const string Admin = "Admin";
public const string Rec_Action = "Recruitment Actions";
}
Custom principal:
public CustomPrincipal(WindowsIdentity source, TBL_UserPermissions baseUser ,bool inDb, string ID) :
base(source)
{
UserPermission = baseUser;
HttpContext.Current.Session["USERID"] = ID;
}
public override bool IsInRole(string role)
{
bool hasPermission = false;
if (UserPermission != null && (UserPermission.UserID != null))
{
hasPermission = db.TBL_UserPermissions.Any(up => up.TBL_Permissions.PermissionName == role && up.IsActive == true && up.UserID == UserPermission.UserID);
}
return (base.IsInRole(role) || (hasPermission));
}
public TBL_UserPermissions UserPermission { get; protected set; }
Upvotes: 0
Views: 207
Reputation: 34793
Server-side, controller/API code can never trust data coming from the client. For a controller method you will have an authenticated user associated with the session. For every request you should assert that the provided ID can be modified by the session user. API methods will include authentication tokens to assert and identify the User in order to determine if they are accessing appropriate records.
If you detect that an ID is coming in that the current user does not have access to, it is an immediate event log notification to administrators about the violation (User ID, record ID, date/time, IP Address, etc.) and terminating the user's session. (Kick to login) The system should track these violations against users and repeated attempts should lock the user's account in the event that their account has been compromised. (Typically it's just a curious bumpkin wondering if something is open/possible)
The same goes for any data coming back with the update request. Everything needs to be validated and only the fields that you allow to be updated should be persisted. This is the #1 reason I advise that you never pass EF entities between client and server. The payload of a request can be tampered with, so code that does an Attach + Modified or Update approach is vulnerable to data tampering.
Upvotes: 0
Reputation: 1395
You need to use some sort of verification, one easy way to do is is to add another number at the end for verification and discard it when you try to acces the page.
For example you and use the sum of digits of the number:
For URL
http://localhost:12997/Manning_HQ/Edit/1274
Sum = 1+2+7+4 =>14 -> 1+4 => 5
The new url will be
http://localhost:12997/Manning_HQ/Edit/12745
Upvotes: 0