Repository navigation
#794 breaks access properties of URL objects #811
Description
Activity
Having upgraded Node-RED to the latest JSONata for the CVE fixes, we're now hitting a variation of this.
The
hasOwnPropertyguard that was added is now blocking access to properties/functions on objects that were previously accessible.We have been forced to update due to the critical CVE, but it is now breaking some users. We're stuck.
@andrew-coleman @mattbaileyuk apologies for tagging you directly, but is this something we can discuss?
To unblock Node-RED, I've forked JSONata and patched it locally to bundle in Node-RED at build time. If there is interest in this, I can PR it back.
Here is the patch I've applied; it maintains the block on accessing the prototype/constructor properties explicitly, as well as other Object functions that can be used to modify things, but reenables access to other properties.
Hi Nick, your suggestion sounds reasonable to me and I think your proposed patch addresses your requirement while still blocking the original security vulnerability. Perhaps we could put your mechanism behind the
allowObjectPropertyAccessguardrail property such that if it's enabled, it uses your 'blocklist' approach, otherwise (default) it blocks all access.Happy to take a PR on that, or if you prefer I can do it.
Tagging @peaktwilight and @c0rydoras who originally reported this vulnerability just to make sure they are happy with this approach.
Thanks!
I'd advocate for an allowlist (for prototypes / properties on prototypes) rather than a denylist/blocklist, e.g. something like:
const allowlist = new Map(); // configurable by/for consumers of the library, should probably default to an empty `Map` (given this is javascript specific configuration, for javascript specific behaviour) allowlist.set(URL.prototype, "*"); // allows almost all properties on `URL` objects (except for `constructor`, see below) allowlist.set(Array.prototype, new Set(["at", "flat"])); // allow `at` and `flat` on arrays
and then, something like
} else if ( input !== null && typeof input === "object" && Object.prototype.hasOwnProperty.call(input, key) && !isFunction(input) ) { return input[key]; } if (typeof input !== "object" || isFunction(input)) return result; const prototype = Object.getPrototypeOf(input); if (!allowlist.has(prototype)) return result; const allowed = allowlist.get(prototype); if ( allowed === "*" && // prototypes generally have a `constructor` property (a.k.a.`Thing.prototype.constructor === Thing`) // see 5. in https://tc39.es/ecma262/multipage/ordinary-and-exotic-objects-behaviours.html#sec-makeconstructor // which probably shouldn't be allowed even if set to "*" key !== "constructor" ) return input[key]; if (allowed.has(key)) return input[key]; return result;
Whilst I appreciate the increased security of an allowlist, it isn't practical for us as Node-RED users could be passing any JavaScript object to the expression, including custom Classes that we have never seen before.
Summary
After upgrading from 2.1.1 to 2.2.0+ (including 2.2.1), JSONata can no longer access properties of JavaScript
URLobjects.This worked correctly in 2.1.1.
Reproduction
Expected (v2.1.1)
Actual (v2.2.0+)
The
URLobject is valid:Possible cause
It appears this change introduced the regression:
Before (v2.1.1):
After (v2.2.0):
URL.hostname,URL.href, etc. are exposed through prototype getters rather than own properties, sohasOwnProperty()returnsfalse.As a result, JSONata no longer resolves these properties.
Question
Was this behavior change intentional?
If so, is there a recommended way to access properties from JavaScript built-in objects such as
URL?