Patch bypass for njs_await_fulfilled, causing UAF again #469
Comments
|
The patch # HG changeset patch
# User Dmitry Volyntsev <xeioex@nginx.com>
# Date 1645208528 0
# Fri Feb 18 18:22:08 2022 +0000
# Node ID d3bd263c19c4e2bbf65ddae3764f3eae6f45648a
# Parent b7a93f20410a99f186ca7c85f7c9187b8212474f
Fixed frame allocation from an awaited frame.
njs_function_frame_save() is used to save the awaited frame when "await"
instruction is encountered. The saving was done as a memcpy() of
existing runtime frame.
njs_function_frame_alloc() is used to alloc a new function frame, this
function tries to use a spare preallocated memory from the previous
frame first. Previously, this function might result in "use-after-free"
when invoked from a restored frame saved with njs_function_frame_save().
Because njs_function_frame_save() left pointers to the spare memory of
the original frame which may be already free when saved frame is
restored.
The fix is to erase fields for the spare memory from the saved frame.
This closes #469 issue on Github.
diff --git a/src/njs_function.c b/src/njs_function.c
--- a/src/njs_function.c
+++ b/src/njs_function.c
@@ -811,9 +811,13 @@ njs_function_frame_save(njs_vm_t *vm, nj
njs_native_frame_t *active, *native;
*frame = *vm->active_frame;
+
frame->previous_active_frame = NULL;
native = &frame->native;
+ native->size = 0;
+ native->free = NULL;
+ native->free_size = 0;
active = &vm->active_frame->native;
value_count = njs_function_frame_value_count(active);
diff --git a/test/js/async_recursive_large.t.js b/test/js/async_recursive_large.t.js
new file mode 100644
--- /dev/null
+++ b/test/js/async_recursive_large.t.js
@@ -0,0 +1,26 @@
+/*---
+includes: [compareArray.js]
+flags: [async]
+---*/
+
+let stages = [];
+
+async function f(v) {
+ if (v == 1000) {
+ return;
+ }
+
+ stages.push(`f>${v}`);
+
+ await "X";
+
+ await f(v + 1);
+
+ stages.push(`f<${v}`);
+}
+
+f(0)
+.then(v => {
+ assert.sameValue(stages.length, 2000);
+})
+.then($DONE, $DONE);
diff --git a/test/js/async_recursive_mid.t.js b/test/js/async_recursive_mid.t.js
--- a/test/js/async_recursive_mid.t.js
+++ b/test/js/async_recursive_mid.t.js
@@ -6,7 +6,7 @@ flags: [async]
let stages = [];
async function f(v) {
- if (v == 3) {
+ if (v == 1000) {
return;
} |
|
@xeioex May I know your plans for a new release including the patch? |
|
Hi @ViieeS, No specific plans yet. Approximately in two weeks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This UAF was introduced in a patch for a similar bug #451, which shows that njs_await_fulfilled is still flawed.
Environment
Proof of concept
Stack dump
Credit
p1umer(@P1umer)
The text was updated successfully, but these errors were encountered: