Skip to content

Commit de1d413

Browse files
kajukitlibartlomieju
authored andcommitted
fix: prevent panic on dynamic import with non-string error name (#32498)
Fixes a panic where a dynamic import rejecting with an Error whose `name` property is a non-string type (Number, Object, Symbol) would panic and terminate the Deno process. --------- Co-authored-by: kaju <kajukitli@users.noreply.github.com>
1 parent 571f704 commit de1d413

File tree

8 files changed

+54
-3
lines changed

8 files changed

+54
-3
lines changed

libs/core/error.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -996,10 +996,14 @@ impl JsError {
996996
let mut out = Vec::with_capacity(arr.length() as usize);
997997

998998
for i in 0..arr.length() {
999-
let key = arr.get_index(scope, i).unwrap();
999+
let Some(key) = arr.get_index(scope, i) else {
1000+
continue;
1001+
};
10001002
let key_name = key.to_rust_string_lossy(scope);
10011003

1002-
let val = exception.get(scope, key).unwrap();
1004+
let Some(val) = exception.get(scope, key) else {
1005+
continue;
1006+
};
10031007
let val_str = val.to_rust_string_lossy(scope);
10041008
out.push((key_name, val_str));
10051009
}

libs/core/runtime/bindings.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,7 +958,8 @@ fn catch_dynamic_import_promise_error<'s, 'i>(
958958
) {
959959
let arg = args.get(0);
960960
if is_instance_of_error(scope, arg) {
961-
let e: crate::error::NativeJsError = serde_v8::from_v8(scope, arg).unwrap();
961+
let e: crate::error::NativeJsError =
962+
serde_v8::from_v8(scope, arg).unwrap_or_default();
962963
let name = e.name.unwrap_or_else(|| {
963964
deno_error::builtin_classes::GENERIC_ERROR.to_string()
964965
});
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"args": "run main.ts",
3+
"output": "main.out"
4+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
caught error
2+
done
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Regression test for GHSA-2f8r-ppr9-ff8f
2+
// Dynamic import with non-string error name should not panic
3+
4+
try {
5+
await import(`data:application/javascript,
6+
const e = new Error("test error");
7+
Object.defineProperty(e, "name", { value: 42 });
8+
throw e;
9+
`);
10+
} catch (e) {
11+
console.log("caught error");
12+
}
13+
14+
console.log("done");
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
{
2+
"args": "run main.ts",
3+
"output": "main.out"
4+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
caught error
2+
done
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Regression test: Error with throwing getter in errorAdditionalPropertyKeys
2+
// should not panic the runtime
3+
4+
const err = new Error("test");
5+
Object.defineProperty(err, Symbol.for("errorAdditionalPropertyKeys"), {
6+
value: ["badProp"],
7+
});
8+
Object.defineProperty(err, "badProp", {
9+
get() {
10+
throw new Error("getter throws");
11+
},
12+
});
13+
14+
try {
15+
throw err;
16+
} catch (e) {
17+
console.log("caught error");
18+
}
19+
20+
console.log("done");

0 commit comments

Comments
 (0)