From 9cec874582b925e373adb0896b85a112702f6664 Mon Sep 17 00:00:00 2001 From: Dmitry Volyntsev Date: Mon, 27 Feb 2023 22:14:34 -0800 Subject: [PATCH] Fixed Error() instance dumping when "name" prop is not primitive. Previously, njs_error_to_string2() might be invoked with error argument pointing to vm->retval. When "name" prop was not primitive vm->retval might be overwritten. As a result error pointer might be referencing a primitive value. In turn the second call of njs_object_property() received an invalid object pointer because it expects only object value types. The fix is to ensure that error object pointer is never overwritten. This closes #615 issue on Github. --- src/njs_array.c | 5 +++-- src/njs_date.c | 5 +++-- src/njs_error.c | 14 ++++++++++++-- src/njs_json.c | 14 +++++++++----- src/njs_object.h | 2 +- src/njs_object_prop.c | 26 ++++++++++++++------------ src/njs_value.c | 2 +- src/test/njs_unit_test.c | 3 +++ 8 files changed, 46 insertions(+), 25 deletions(-) diff --git a/src/njs_array.c b/src/njs_array.c index ec21e60c..0eb3276f 100644 --- a/src/njs_array.c +++ b/src/njs_array.c @@ -1404,10 +1404,11 @@ njs_array_prototype_to_string(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, static const njs_value_t join_string = njs_string("join"); - if (njs_is_object(&args[0])) { + if (njs_is_object(njs_argument(args, 0))) { njs_object_property_init(&lhq, &join_string, NJS_JOIN_HASH); - ret = njs_object_property(vm, &args[0], &lhq, &value); + ret = njs_object_property(vm, njs_object(njs_argument(args, 0)), &lhq, + &value); if (njs_slow_path(ret == NJS_ERROR)) { return ret; diff --git a/src/njs_date.c b/src/njs_date.c index e4772ce4..462a7139 100644 --- a/src/njs_date.c +++ b/src/njs_date.c @@ -1385,10 +1385,11 @@ njs_date_prototype_to_json(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, static const njs_value_t to_iso_string = njs_string("toISOString"); - if (njs_is_object(&args[0])) { + if (njs_is_object(njs_argument(args, 0))) { njs_object_property_init(&lhq, &to_iso_string, NJS_TO_ISO_STRING_HASH); - ret = njs_object_property(vm, &args[0], &lhq, &value); + ret = njs_object_property(vm, njs_object(njs_argument(args, 0)), &lhq, + &value); if (njs_slow_path(ret == NJS_ERROR)) { return ret; diff --git a/src/njs_error.c b/src/njs_error.c index 906dfd48..33980203 100644 --- a/src/njs_error.c +++ b/src/njs_error.c @@ -574,6 +574,7 @@ njs_error_to_string2(njs_vm_t *vm, njs_value_t *retval, size_t length; u_char *p; njs_int_t ret; + njs_object_t *error_object; njs_value_t value1, value2; njs_value_t *name_value, *message_value; njs_string_prop_t name, message; @@ -581,6 +582,8 @@ njs_error_to_string2(njs_vm_t *vm, njs_value_t *retval, static const njs_value_t default_name = njs_string("Error"); + njs_assert(njs_is_object(error)); + if (want_stack) { ret = njs_error_stack(vm, njs_value_arg(error), retval); if (njs_slow_path(ret == NJS_ERROR)) { @@ -592,9 +595,11 @@ njs_error_to_string2(njs_vm_t *vm, njs_value_t *retval, } } + error_object = njs_object(error); + njs_object_property_init(&lhq, &njs_string_name, NJS_NAME_HASH); - ret = njs_object_property(vm, error, &lhq, &value1); + ret = njs_object_property(vm, error_object, &lhq, &value1); if (njs_slow_path(ret == NJS_ERROR)) { return ret; @@ -616,7 +621,7 @@ njs_error_to_string2(njs_vm_t *vm, njs_value_t *retval, lhq.key_hash = NJS_MESSAGE_HASH; lhq.key = njs_str_value("message"); - ret = njs_object_property(vm, error, &lhq, &value2); + ret = njs_object_property(vm, error_object, &lhq, &value2); if (njs_slow_path(ret == NJS_ERROR)) { return ret; @@ -686,6 +691,11 @@ njs_error_prototype_to_string(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_int_t njs_error_to_string(njs_vm_t *vm, njs_value_t *retval, const njs_value_t *error) { + if (njs_slow_path(!njs_is_object(error))) { + njs_type_error(vm, "\"error\" is not an object"); + return NJS_ERROR; + } + return njs_error_to_string2(vm, retval, error, 1); } diff --git a/src/njs_json.c b/src/njs_json.c index 0b7d1cb8..6e53286c 100644 --- a/src/njs_json.c +++ b/src/njs_json.c @@ -1252,15 +1252,19 @@ njs_object_to_json_function(njs_vm_t *vm, njs_value_t *value) static const njs_value_t to_json_string = njs_string("toJSON"); - njs_object_property_init(&lhq, &to_json_string, NJS_TO_JSON_HASH); + if (njs_is_object(value)) { + njs_object_property_init(&lhq, &to_json_string, NJS_TO_JSON_HASH); - ret = njs_object_property(vm, value, &lhq, &retval); + ret = njs_object_property(vm, njs_object(value), &lhq, &retval); - if (njs_slow_path(ret == NJS_ERROR)) { - return NULL; + if (njs_slow_path(ret == NJS_ERROR)) { + return NULL; + } + + return njs_is_function(&retval) ? njs_function(&retval) : NULL; } - return njs_is_function(&retval) ? njs_function(&retval) : NULL; + return NULL; } diff --git a/src/njs_object.h b/src/njs_object.h index 98823142..35b098c1 100644 --- a/src/njs_object.h +++ b/src/njs_object.h @@ -98,7 +98,7 @@ njs_int_t njs_prop_private_copy(njs_vm_t *vm, njs_property_query_t *pq, njs_object_t *proto); njs_object_prop_t *njs_object_prop_alloc(njs_vm_t *vm, const njs_value_t *name, const njs_value_t *value, uint8_t attributes); -njs_int_t njs_object_property(njs_vm_t *vm, const njs_value_t *value, +njs_int_t njs_object_property(njs_vm_t *vm, njs_object_t *object, njs_lvlhsh_query_t *lhq, njs_value_t *retval); njs_object_prop_t *njs_object_property_add(njs_vm_t *vm, njs_value_t *object, njs_value_t *key, njs_bool_t replace); diff --git a/src/njs_object_prop.c b/src/njs_object_prop.c index d829e1c2..6bf1c280 100644 --- a/src/njs_object_prop.c +++ b/src/njs_object_prop.c @@ -91,15 +91,13 @@ njs_object_prop_alloc2(njs_vm_t *vm, const njs_value_t *name, njs_int_t -njs_object_property(njs_vm_t *vm, const njs_value_t *value, - njs_lvlhsh_query_t *lhq, njs_value_t *retval) +njs_object_property(njs_vm_t *vm, njs_object_t *object, njs_lvlhsh_query_t *lhq, + njs_value_t *retval) { njs_int_t ret; - njs_object_t *object; + njs_value_t value; njs_object_prop_t *prop; - object = njs_object(value); - do { ret = njs_lvlhsh_find(&object->hash, lhq); @@ -135,7 +133,9 @@ found: return NJS_OK; } - return njs_function_apply(vm, njs_prop_getter(prop), value, 1, retval); + njs_set_object(&value, object); + + return njs_function_apply(vm, njs_prop_getter(prop), &value, 1, retval); } @@ -720,6 +720,7 @@ njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name, njs_int_t ret; njs_bool_t data, accessor; njs_value_t value; + njs_object_t *desc_object; njs_function_t *getter, *setter; njs_object_prop_t *prop; njs_lvlhsh_query_t lhq; @@ -741,10 +742,11 @@ njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name, accessor = 0; getter = NJS_PROP_PTR_UNSET; setter = NJS_PROP_PTR_UNSET; + desc_object = njs_object(desc); njs_object_property_init(&lhq, &get_string, NJS_GET_HASH); - ret = njs_object_property(vm, desc, &lhq, &value); + ret = njs_object_property(vm, desc_object, &lhq, &value); if (njs_slow_path(ret == NJS_ERROR)) { return NULL; } @@ -762,7 +764,7 @@ njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name, lhq.key = njs_str_value("set"); lhq.key_hash = NJS_SET_HASH; - ret = njs_object_property(vm, desc, &lhq, &value); + ret = njs_object_property(vm, desc_object, &lhq, &value); if (njs_slow_path(ret == NJS_ERROR)) { return NULL; } @@ -780,7 +782,7 @@ njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name, lhq.key = njs_str_value("value"); lhq.key_hash = NJS_VALUE_HASH; - ret = njs_object_property(vm, desc, &lhq, &value); + ret = njs_object_property(vm, desc_object, &lhq, &value); if (njs_slow_path(ret == NJS_ERROR)) { return NULL; } @@ -793,7 +795,7 @@ njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name, lhq.key = njs_str_value("writable"); lhq.key_hash = NJS_WRITABABLE_HASH; - ret = njs_object_property(vm, desc, &lhq, &value); + ret = njs_object_property(vm, desc_object, &lhq, &value); if (njs_slow_path(ret == NJS_ERROR)) { return NULL; } @@ -812,7 +814,7 @@ njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name, lhq.key = njs_str_value("enumerable"); lhq.key_hash = NJS_ENUMERABLE_HASH; - ret = njs_object_property(vm, desc, &lhq, &value); + ret = njs_object_property(vm, desc_object, &lhq, &value); if (njs_slow_path(ret == NJS_ERROR)) { return NULL; } @@ -824,7 +826,7 @@ njs_descriptor_prop(njs_vm_t *vm, const njs_value_t *name, lhq.key = njs_str_value("configurable"); lhq.key_hash = NJS_CONFIGURABLE_HASH; - ret = njs_object_property(vm, desc, &lhq, &value); + ret = njs_object_property(vm, desc_object, &lhq, &value); if (njs_slow_path(ret == NJS_ERROR)) { return NULL; } diff --git a/src/njs_value.c b/src/njs_value.c index ff885fe3..296f057a 100644 --- a/src/njs_value.c +++ b/src/njs_value.c @@ -157,7 +157,7 @@ njs_value_to_primitive(njs_vm_t *vm, njs_value_t *dst, njs_value_t *value, lhq.key_hash = hashes[hint]; lhq.key = names[hint]; - ret = njs_object_property(vm, value, &lhq, &method); + ret = njs_object_property(vm, njs_object(value), &lhq, &method); if (njs_slow_path(ret == NJS_ERROR)) { return ret; diff --git a/src/test/njs_unit_test.c b/src/test/njs_unit_test.c index c7db56b6..be627dbc 100644 --- a/src/test/njs_unit_test.c +++ b/src/test/njs_unit_test.c @@ -22849,6 +22849,9 @@ static njs_unit_test_t njs_shell_test[] = "sq(function () { return 3 })" ENTER), njs_str("9") }, + { njs_str("var e = Error(); e.name = {}; e" ENTER), + njs_str("[object Object]") }, + /* Temporary indexes */ { njs_str("var a = [1,2,3], i; for (i in a) {Object.seal({});}" ENTER), -- 2.47.3