Switch obj.hasOwnProperty() to Object.hasOwn() where the receiver is untrusted

[thehabes]

Summary

Eight call sites invoke hasOwnProperty as a method on the object being tested. RERUM stores arbitrary JSON, so an object under test can carry its own hasOwnProperty property, which shadows Object.prototype.hasOwnProperty and makes the call fail.

Why Object.hasOwn is immune

It is about which object is the receiver of the method lookup, not the method name.

• obj.hasOwnProperty(k) resolves hasOwnProperty on obj — own property first, then up the prototype chain. An own key named hasOwnProperty shadows the built-in.
• Object.hasOwn(obj, k) resolves hasOwn on the Object constructor. obj is an ordinary argument and is never consulted to find the function.

--- obj.hasOwnProperty("__deleted")   (receiver is the record) ---
plain record             -> true
record w/ hasOwnProperty -> THROWS: TypeError: obj.hasOwnProperty is not a function
record w/ hasOwn         -> true
record w/ fn override    -> false          <-- silently wrong

--- Object.hasOwn(obj, "__deleted")   (receiver is Object) ---
plain record             -> true
record w/ hasOwnProperty -> true
record w/ hasOwn         -> true
record w/ fn override    -> true

The failure mode depends on the value. A string throws a TypeError. A function returning false would not throw at all — a deleted record would be reported as live. That second case is unreachable here, because RERUM stores JSON and JSON cannot encode a function, so the realistic worst case is a 500.

Call sites

#	Site	Receiver	Provenance	Verdict
1	utils.js:46	received_options	request body	switch — highest priority
2	utils.js:72	received_options	request body	switch — highest priority
3	utils.js:105 (isDeleted)	obj	stored record	switch
4	utils.js:113 (isReleased)	obj	stored record	switch
5	utils.js:114 (isReleased)	obj.__rerum	RERUM-minted, fixed shape	consistency only
6	controllers/patchUpdate.js:59	originalObject	stored record	switch
7	controllers/patchUnset.js:65	originalObject	stored record	switch
8	controllers/patchSet.js:63	originalObject	stored record	switch

Sites 1 and 2 are a different class from the rest

Sites 3–8 require a poisoned record to already exist in the database. Sites 1 and 2 take their receiver straight from the request:

--- utils.js:46/72  configureRerumOptions ---
  OK     normal create body
  BREAKS body w/ __rerum.hasOwnProperty -> TypeError: received_options.hasOwnProperty is not a function

create() does provided = structuredClone(req.body), and configureRerumOptions() clones provided.__rerum into received_options. structuredClone preserves an own hasOwnProperty key, so POST /v1/api/create with a body of {"__rerum":{"hasOwnProperty":"x"}} throws. The call is at controllers/crud.js:41, before the try at line 54, so it escapes as a 500 rather than a 400. bulkCreate has the same shape through controllers/bulk.js:73.

Both require a valid token, so this is an authenticated self-inflicted 500 rather than an anonymous availability problem. It is still a 500 on well-formed JSON, which is the wrong answer to give.

putUpdate.js:115 is not affected — it passes extUpdate=true, and that branch sets received_options = {} before either check runs.

Site 5 is the only genuine non-issue

configureRerumOptions() builds rerumOptions from scratch and never spreads caller keys into it, so a stored __rerum always has the fixed RERUM shape. obj.__rerum.hasOwnProperty(...) only breaks if isReleased is handed a hand-made object, and all twelve callers of isDeleted/isReleased pass a record from db.findOne. Worth switching with the others so the next reader does not have to re-derive that it is safe.

Current impact: none

Measured against production (annotationStore.alpha):

'hasOwnProperty' present on stored records:                  0
'__rerum.hasOwnProperty' present on stored records:          0
'__rerum.history.hasOwnProperty' present on stored records:  0
'__rerum.releases.hasOwnProperty' present on stored records: 0

Also checked and absent at the top level: constructor, __proto__, toString, valueOf, isPrototypeOf, propertyIsEnumerable.

Nothing stored today triggers any of this. Sites 1 and 2 do not depend on stored data, so those are reachable now by any client holding a token.

Suggested fix

Replace each site with Object.hasOwn(receiver, key):

// utils.js
const isDeleted = function(obj){
    return Object.hasOwn(obj, "__deleted")
}

const isReleased = function(obj){
    let bool =
    (Object.hasOwn(obj, "__rerum") &&
        Object.hasOwn(obj.__rerum, "isReleased") &&
        obj.__rerum.isReleased !== "")
    return bool
}

Eight lines across utils.js, controllers/patchUpdate.js, controllers/patchUnset.js, and controllers/patchSet.js. No behavior change for valid data. Object.hasOwn is available from Node 16.9, well under the repo's >=24.14.0 engine floor.

The older equivalent is Object.prototype.hasOwnProperty.call(obj, k), immune for the same reason.

[thehabes]

A second source for this hazard: /v1/id/:_id/expanded manufactures the poisoned receivers

Split out of the static review of #286. This is not another call site — it is where the objects that break the call sites above can now come from.

This issue's table assumes a poisoned record already exists in the database. The expand endpoint added in #286 can produce one from ordinary, well-formed inputs.

What happens

PROTECTED_EXPANSION_KEYS in controllers/utils.js guards __proto__, which is the name that can actually reassign a prototype. The sibling names that merely shadow Object.prototype methods are not guarded, so an Annotation body asserting one of them merges it onto the entity as an own property.

Observed live against annotationStore.alpha, GET /v1/id/6a760abbeaaeae59c6c5e3ba/expanded:

"constructor": { "polluted": "yes" },
"toString": "polluted",
"toJSON": "polluted",
"prototype": "polluted",
"hasOwnProperty": "polluted",
"valueOf": "polluted"

The __proto__ probe on the same entity is correctly dropped.

Why it belongs on this issue

Nothing inside the endpoint breaks. I checked the two candidates:

• toJSON cannot become a function, because BSON/JSON cannot encode one, so res.json() is unaffected.
• applyExpansionAnnotations() uses Object.hasOwn, so the merge loop does not trip over its own output — that one is already correct.

The cost is downstream, and it is exactly the failure this issue describes:

• A client doing expanded.hasOwnProperty(k) on the response throws.
• A client that round-trips an expanded record back through POST /v1/api/create stores a record carrying an own hasOwnProperty. Every later PUT/PATCH on that record then reaches sites 6–8 in the table above with a poisoned receiver.

So /expanded turns "a poisoned record would have to already exist" into "a poisoned record can be produced by two ordinary API calls." Switching the call sites is still the right fix for the sites; this is about not minting the receivers in the first place.

Suggested fix, on the merge side

/**
 * Identity and system properties an Annotation body must never overwrite when its assertions are
 * merged onto an entity.  The JavaScript intrinsics are here because a stored record carrying one
 * as an own property breaks any consumer that calls it as a method on the record.
 */
const PROTECTED_EXPANSION_KEYS = new Set([
    "@id", "id", "_id", "__rerum", "__deleted",
    "__proto__", "constructor", "prototype", "hasOwnProperty", "toString", "valueOf", "toJSON"
])

public/API.html lists the protected set under Entity Expansion ("An Annotation body asserting @id, id, _id, __rerum, __deleted, or __proto__ contributes nothing") and would need the same names so the prose and the Set stay in step.

Note these names are also not covered by the neighbouring design statement in that section — "Linked Data keywords that describe the record rather than identify it are not held back," which is about type, @type, and @context. hasOwnProperty describes nothing; it is a JavaScript artifact.

Current exposure

Production holds 6 leaf Annotations with such a body key, and all six are the intentional PR286-PROBE fixtures created to exercise this path. No real data is affected today.

The count headers would not move if this is fixed: those Annotations still carry a mergeable assertion, and Annotations-Merged counts contribution rather than acceptance — which API.html already states.