User Controlled Ownership Field
ID |
csharp.user_controlled_ownership_field |
Severity |
high |
Remediation Complexity |
medium |
Remediation Risk |
medium |
Remediation Effort |
medium |
Resource |
Access Control |
Language |
CSharp |
Tags |
ASVS50:v8.1.1, CWE:639, CWE:915, NIST.SP.800-53, OWASP:2025:A01, PCI-DSS:6.5.8 |
Description
Data that came from the request reaches the property of a persisted entity that says whose record it is — the customer id, the owner id, the tenant id, whatever this application calls it. The caller therefore decides which account a record belongs to: they can create a record under somebody else’s name, or move one of somebody else’s to themselves.
The detector reports four ways the value gets there:
-
the property set,
order.CustomerId = Request.QueryString["customerId"]; -
the object initializer,
new Order { CustomerId = id }, where the write is part of the creation expression rather than a statement of its own; -
the call named after the field,
order.SetCustomerId(id); -
a binder that copies request data into the entity wholesale —
JsonSerializer.Deserialize<Order>(body),JsonConvert.PopulateObject(body, order),mapper.Map<Order>(dto)— where the field is never named at all, because the binder writes every property the entity has.
Rationale
This is CWE-915 (mass assignment) read as CWE-639 (authorization bypass through a user-controlled key): the property being assigned is not just any attribute, it is the one the authorization decision is later made on. Every subsequent "is this record yours?" check answers yes, because the request already said so.
Mined from the code |
That the value is request-derived, and that it reaches the write: both are decided by the tainting analysis against the shipped library metadata — the request accessors and model attributes of the framework in use. No profile is involved. |
Needs a profile fact |
Which property denotes the entity’s owner. Nothing in a rule catalogue can express it, because
the next application calls the property something else; the application’s own convention profile
names it ( |
Without that fact this rule has no sink and reports nothing, and says so in the scan log rather
than reporting a clean result it could not establish. Every finding carries the fact it used and the
file:line where that fact can be checked.
The boundary with the API Security mass-assignment detector is the provenance of the field name:
that detector keeps the generic vocabulary (admin, role, scopes, balance) decided from the
endpoint surface, and this one keeps the property the application’s profile named, with a flow
proving the value reaches the entity.
What the detector cannot see
A binder that takes the request implicitly rather than as an argument — TryUpdateModelAsync(order)
with no value provider — passes no request-derived value into the call, so there is no flow for the
analysis to follow and the write is not reported. Where the binder is handed the body or a value
provider, as in TryUpdateModelAsync(order, form), it is.
Remediation
Never let the request choose the owner. Set the property from the authenticated principal on the server side:
[HttpPost("/orders")]
public async Task<Order> Create([FromBody] OrderForm form)
{
var order = new Order
{
Description = form.Description,
CustomerId = CurrentCustomerId(User) // server-derived, not from the request
};
return await orders.SaveAsync(order);
}Where a binder is used, keep the owner property off the bindable surface: a dedicated request DTO
that simply has no such property, [BindNever] on the entity’s property, an explicit allow list
([Bind(nameof(Order.Description))], or the includeExpressions overload of
TryUpdateModelAsync), or [JsonIgnore] for the serializer. Giving the property a private setter
removes the assignment shapes outright. Validating the submitted value against the caller is a
weaker fix than not accepting it, but it is a fix — reject the request when the submitted owner is
not the caller.