From ce0dbb7b8fe4c83996320c0756c9338b8bdcffb7 Mon Sep 17 00:00:00 2001 From: Fabrice Bellard Date: Tue, 29 Sep 2026 14:55:55 +0200 Subject: [PATCH] fix Proxy getOwnPropertyDescriptor invariant check for values and accessors --- quickjs.c | 75 +++++++++++++++++++++++++++++----------------- test262_errors.txt | 1 - 2 files changed, 48 insertions(+), 28 deletions(-) diff --git a/quickjs.c b/quickjs.c index 19c05ed..8b3a450 100644 --- a/quickjs.c +++ b/quickjs.c @@ -8835,7 +8835,7 @@ int JS_GetOwnPropertyNames(JSContext *ctx, JSPropertyEnum **ptab, /* Return -1 if exception, FALSE if the property does not exist, TRUE if it exists. If TRUE is - returned, the property descriptor 'desc' is filled present. */ + returned, the property descriptor 'desc' is filled. */ static int JS_GetOwnPropertyInternal(JSContext *ctx, JSPropertyDescriptor *desc, JSObject *p, JSAtom prop) { @@ -51150,6 +51150,35 @@ static JSValue js_create_desc(JSContext *ctx, JSValueConst val, return ret; } +/* return FALSE if not OK */ +static BOOL check_define_prop_desc(JSContext *ctx, const JSPropertyDescriptor *desc, + JSValueConst val, JSValueConst getter, JSValueConst setter, + int flags) +{ + if (!check_define_prop_flags(desc->flags, flags)) + return FALSE; + + /* do the missing check from check_define_prop_flags() */ + if (!(desc->flags & JS_PROP_CONFIGURABLE)) { + if ((desc->flags & JS_PROP_TMASK) == JS_PROP_GETSET) { + if ((flags & JS_PROP_HAS_GET) && + !js_same_value(ctx, getter, desc->getter)) { + return FALSE; + } + if ((flags & JS_PROP_HAS_SET) && + !js_same_value(ctx, setter, desc->setter)) { + return FALSE; + } + } else if (!(desc->flags & JS_PROP_WRITABLE)) { + if ((flags & JS_PROP_HAS_VALUE) && + !js_same_value(ctx, val, desc->value)) { + return FALSE; + } + } + } + return TRUE; +} + static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc, JSValueConst obj, JSAtom prop) { @@ -51187,11 +51216,12 @@ 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) + BOOL tmp; + tmp = !(target_desc.flags & JS_PROP_CONFIGURABLE) || !p->extensible; + js_free_desc(ctx, &target_desc); + if (tmp) goto fail; } ret = FALSE; @@ -51200,12 +51230,17 @@ 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); + if (target_desc_ret) + js_free_desc(ctx, &target_desc); return -1; } res = js_obj_to_desc(ctx, &result_desc, trap_result_obj); JS_FreeValue(ctx, trap_result_obj); - if (res < 0) + if (res < 0) { + if (target_desc_ret) + js_free_desc(ctx, &target_desc); return -1; + } /* convert the result_desc.flags to property flags */ if (result_desc.flags & (JS_PROP_HAS_GET | JS_PROP_HAS_SET)) { @@ -51222,10 +51257,10 @@ static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc 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)) + if (!check_define_prop_desc(ctx, &target_desc, result_desc.value, + result_desc.getter, result_desc.setter, flags1)) { goto fail1; + } } else { if (!extensible_target) goto fail1; @@ -51240,11 +51275,15 @@ static int js_proxy_get_own_property(JSContext *ctx, JSPropertyDescriptor *pdesc /* proxy-missing-checks */ fail1: js_free_desc(ctx, &result_desc); + if (target_desc_ret) + js_free_desc(ctx, &target_desc); fail: JS_ThrowTypeError(ctx, "proxy: inconsistent getOwnPropertyDescriptor"); return -1; } } + if (target_desc_ret) + js_free_desc(ctx, &target_desc); ret = TRUE; if (pdesc) { *pdesc = result_desc; @@ -51313,26 +51352,8 @@ static int js_proxy_define_own_property(JSContext *ctx, JSValueConst obj, if (!p->extensible || setting_not_configurable) goto fail; } else { - if (!check_define_prop_flags(desc.flags, flags)) + if (!check_define_prop_desc(ctx, &desc, val, getter, setter, flags)) goto fail1; - /* do the missing check from check_define_prop_flags() */ - if (!(desc.flags & JS_PROP_CONFIGURABLE)) { - if ((desc.flags & JS_PROP_TMASK) == JS_PROP_GETSET) { - if ((flags & JS_PROP_HAS_GET) && - !js_same_value(ctx, getter, desc.getter)) { - goto fail1; - } - if ((flags & JS_PROP_HAS_SET) && - !js_same_value(ctx, setter, desc.setter)) { - goto fail1; - } - } else if (!(desc.flags & JS_PROP_WRITABLE)) { - if ((flags & JS_PROP_HAS_VALUE) && - !js_same_value(ctx, val, desc.value)) { - goto fail1; - } - } - } /* additional checks */ if ((desc.flags & JS_PROP_CONFIGURABLE) && setting_not_configurable) diff --git a/test262_errors.txt b/test262_errors.txt index 2e84de7..386b674 100644 --- a/test262_errors.txt +++ b/test262_errors.txt @@ -13,5 +13,4 @@ test262/test/staging/sm/lexical-environment/block-scoped-functions-deprecated-re test262/test/staging/sm/lexical-environment/var-in-catch-body-annex-b-eval.js:16: Test262Error: Expected SameValue(«"g"», «"global-x"») to be true test262/test/staging/sm/object/defineProperties-order.js:11: Test262Error: Expected SameValue(«"ownKeys,getOwnPropertyDescriptor,getOwnPropertyDescriptor,get,get"», «"ownKeys,getOwnPropertyDescriptor,get,getOwnPropertyDescriptor,get"») to be true test262/test/staging/sm/regress/regress-602621.js:13: Test262Error: function sub-statement must override arguments Expected SameValue(«"function"», «"object"») to be true -test262/test/staging/sm/regress/regress-1383630.js:28: Test262Error: proxy must report the same value for the non-writable, non-configurable property Expected a TypeError to be thrown but no exception was thrown at all test262/test/staging/sm/statements/arrow-function-in-for-statement-head.js:13: Test262Error: Expected a SyntaxError to be thrown but no exception was thrown at all -- 2.47.3