From 5563c84e8a9ab2dbead9a3378eb9a78bea1d45ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Tue, 4 Aug 2026 14:28:53 +0200 Subject: [PATCH] Fix the descriptor checks of the Proxy getOwnPropertyDescriptor trap Fixes #1629. --- quickjs.c | 70 +++++++++++++++++++++++++++++------------ tests/bug1629.js | 81 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 131 insertions(+), 20 deletions(-) create mode 100644 tests/bug1629.js diff --git a/quickjs.c b/quickjs.c index bbac00c33..030471971 100644 --- a/quickjs.c +++ b/quickjs.c @@ -52411,7 +52411,8 @@ static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc { JSProxyData *s; JSValue method, trap_result_obj, prop_val; - int res, target_desc_ret, ret; + int res, ret; + int target_desc_ret = false; JSObject *p; JSValueConst args[2]; JSPropertyDescriptor result_desc, target_desc; @@ -52443,12 +52444,10 @@ static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc JS_FreeValue(ctx, trap_result_obj); return -1; } - if (target_desc_ret) - js_free_desc(ctx, &target_desc); if (JS_IsUndefined(trap_result_obj)) { if (target_desc_ret) { if (!(target_desc.flags & JS_PROP_CONFIGURABLE) || !p->extensible) - goto fail; + goto fail_free_target; } ret = false; } else { @@ -52456,24 +52455,39 @@ static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc extensible_target = JS_IsExtensible(ctx, s->target); if (extensible_target < 0) { JS_FreeValue(ctx, trap_result_obj); - return -1; + goto exception; } res = js_obj_to_desc(ctx, &result_desc, trap_result_obj); JS_FreeValue(ctx, trap_result_obj); if (res < 0) - return -1; + goto exception; + /* IsCompatiblePropertyDescriptor(): the descriptor returned by + the trap is completed first, i.e. the missing fields take + their default value (undefined for the value and the + accessors, false for the attributes) */ if (target_desc_ret) { /* convert result_desc.flags to defineProperty flags */ flags1 = result_desc.flags | JS_PROP_HAS_CONFIGURABLE | JS_PROP_HAS_ENUMERABLE; - if (result_desc.flags & JS_PROP_GETSET) + if (result_desc.flags & (JS_PROP_HAS_GET | JS_PROP_HAS_SET)) flags1 |= JS_PROP_HAS_GET | JS_PROP_HAS_SET; else flags1 |= JS_PROP_HAS_VALUE | JS_PROP_HAS_WRITABLE; - /* XXX: not complete check: need to compare value & - getter/setter as in defineproperty */ if (!check_define_prop_flags(target_desc.flags, flags1)) goto fail1; + /* a non-configurable property cannot be reported with + different accessors, nor with a different value if it is + not writable either */ + if (!(target_desc.flags & JS_PROP_CONFIGURABLE)) { + if ((target_desc.flags & JS_PROP_TMASK) == JS_PROP_GETSET) { + if (!js_same_value(ctx, result_desc.getter, target_desc.getter) || + !js_same_value(ctx, result_desc.setter, target_desc.setter)) + goto fail1; + } else if (!(target_desc.flags & JS_PROP_WRITABLE)) { + if (!js_same_value(ctx, result_desc.value, target_desc.value)) + goto fail1; + } + } } else { if (!extensible_target) goto fail1; @@ -52481,26 +52495,42 @@ static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc if (!(result_desc.flags & JS_PROP_CONFIGURABLE)) { if (!target_desc_ret || (target_desc.flags & JS_PROP_CONFIGURABLE)) goto fail1; - if ((result_desc.flags & - (JS_PROP_GETSET | JS_PROP_WRITABLE)) == 0 && - target_desc_ret && - (target_desc.flags & JS_PROP_WRITABLE) != 0) { - /* proxy-missing-checks */ - fail1: - js_free_desc(ctx, &result_desc); - fail: - JS_ThrowTypeError(ctx, "proxy: inconsistent getOwnPropertyDescriptor"); - return -1; - } + /* a writable property of the target cannot be reported as + non-configurable and non-writable */ + if ((result_desc.flags & (JS_PROP_HAS_GET | JS_PROP_HAS_SET)) == 0 && + !(result_desc.flags & JS_PROP_WRITABLE) && + (target_desc.flags & JS_PROP_WRITABLE)) + goto fail1; } ret = true; if (pdesc) { + /* the returned descriptor must use the same flags + convention as JS_GetOwnPropertyInternal(), not the + JS_PROP_HAS_xxx one used by js_obj_to_desc() */ + if (result_desc.flags & (JS_PROP_HAS_GET | JS_PROP_HAS_SET)) + result_desc.flags = (result_desc.flags & JS_PROP_C_W_E) | JS_PROP_GETSET; + else + result_desc.flags &= JS_PROP_C_W_E; *pdesc = result_desc; } else { js_free_desc(ctx, &result_desc); } } + if (target_desc_ret) + js_free_desc(ctx, &target_desc); return ret; + fail1: + js_free_desc(ctx, &result_desc); + fail_free_target: + if (target_desc_ret) + js_free_desc(ctx, &target_desc); + fail: + JS_ThrowTypeError(ctx, "proxy: inconsistent getOwnPropertyDescriptor"); + return -1; + exception: + if (target_desc_ret) + js_free_desc(ctx, &target_desc); + return -1; } static int js_proxy_define_own_property(JSContext *ctx, JSValueConst obj, diff --git a/tests/bug1629.js b/tests/bug1629.js new file mode 100644 index 000000000..6b78c5bbc --- /dev/null +++ b/tests/bug1629.js @@ -0,0 +1,81 @@ +import { assert, assertThrows } from "./assert.js"; + +// The descriptor returned by the "getOwnPropertyDescriptor" trap is +// completed and then checked with IsCompatiblePropertyDescriptor() +// against the target's own property. + +function proxy(target, desc) { + return new Proxy(target, { getOwnPropertyDescriptor() { return desc; } }); +} + +function gopd(target, desc) { + return Object.getOwnPropertyDescriptor(proxy(target, desc), "x"); +} + +const g = function() {}; +const g2 = function() {}; +const s = function() {}; + +// Non-configurable, non-writable data property: the value cannot differ. +{ + const t = Object.defineProperty({}, "x", { value: 1 }); + assertThrows(TypeError, () => gopd(t, { value: 2 })); + // A completed descriptor always has a value, so an empty one reports + // "undefined" and is incompatible too. + assertThrows(TypeError, () => gopd(t, {})); + assertThrows(TypeError, () => gopd(t, { value: 1, enumerable: true })); + // Reporting the same value is fine. + const d = gopd(t, { value: 1 }); + assert(d.value, 1); + assert(d.writable, false); + assert(d.enumerable, false); + assert(d.configurable, false); +} + +// Non-configurable but writable: the value may differ, but the property +// cannot be reported as non-writable. +{ + const t = Object.defineProperty({}, "x", { value: 1, writable: true }); + assert(gopd(t, { value: 2, writable: true }).value, 2); + assertThrows(TypeError, () => gopd(t, { value: 1 })); +} + +// A non-configurable accessor cannot be reported with other accessors. +{ + const t = Object.defineProperty({}, "x", { get: g, set: s }); + assertThrows(TypeError, () => gopd(t, { get: g2, set: s })); + assertThrows(TypeError, () => gopd(t, { get: g, set: g2 })); + assertThrows(TypeError, () => gopd(t, { get: g })); + assertThrows(TypeError, () => gopd(t, { value: 1 })); + const d = gopd(t, { get: g, set: s }); + assert(d.get, g); + assert(d.set, s); +} + +// A non-configurable data property cannot be reported as an accessor. +{ + const t = Object.defineProperty({}, "x", { value: 1 }); + assertThrows(TypeError, () => gopd(t, { get: g })); +} + +// A configurable property of the target puts no constraint on the value, +// but the trap still cannot report it as non-configurable. +{ + const t = Object.defineProperty({}, "x", { value: 1, configurable: true }); + assert(gopd(t, { value: 2, configurable: true }).value, 2); + assertThrows(TypeError, () => gopd(t, { value: 2 })); +} + +// An accessor descriptor accepted by the checks is reported as such and +// not silently turned into a data descriptor. +{ + const d = gopd({}, { get: g, set: s, configurable: true }); + assert(d.get, g); + assert(d.set, s); + assert("value" in d, false); + assert("writable" in d, false); + + const p = proxy({}, { get: g, set: s, configurable: true }); + assert(p.__lookupGetter__("x"), g); + assert(p.__lookupSetter__("x"), s); +}