Fixed potential memory corruption in njs_function_frame_invoke().

It is a bug to cast vm->top_frame which is (njs_native_frame_t *) type,
to (njs_frame_t *) in general context.  The cast is allowed only for the
lambda frames.

The bug did not manifest itself previously because native frames were
allocated with NJS_NATIVE_FRAME_SIZE, which is sizeof(njs_value_t) aligned
size of njs_native_frame_t (on 64bit platform are 80 and 72 bytes
respectively). njs_frame_t contains njs_native_frame_t as a first field.
The frame->retval assignment for native frames resulted in 8 padding bytes
were used as a storage space for the retval field.

The bug becomes visible when the size of njs_native_frame_t changes.

The issue was introduced in 540f03725df2.

diff --git a/src/njs_function.c b/src/njs_function.c
index a50cde4339534044ce59afbc1f72780a04d01002..66a70fad1469ea0bea6ae170ca84837b8a4a2c6d 100644 (file)
--- a/src/njs_function.c
+++ b/src/njs_function.c
@@ -677,13 +677,11 @@ njs_function_native_call(njs_vm_t *vm)
 {
     njs_int_t              ret;
     njs_value_t            *value;
-    njs_frame_t            *frame;
     njs_function_t         *function, *target;
     njs_native_frame_t     *native, *previous;
     njs_function_native_t  call;
 
     native = vm->top_frame;
-    frame = (njs_frame_t *) native;
     function = native->function;
 
     if (njs_fast_path(function->bound == NULL)) {
@@ -711,10 +709,10 @@ njs_function_native_call(njs_vm_t *vm)
 
     previous = njs_function_previous_frame(native);
 
-    njs_vm_scopes_restore(vm, frame, previous);
+    njs_vm_scopes_restore(vm, native, previous);
 
     if (!native->skip) {
-        value = njs_vmcode_operand(vm, frame->retval);
+        value = njs_vmcode_operand(vm, native->retval);
         /*
          * GC: value external/internal++ depending
          * on vm->retval and retval type
@@ -1035,10 +1033,10 @@ static njs_int_t
 njs_function_prototype_call(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     njs_index_t unused)
 {
-    njs_int_t          ret;
-    njs_frame_t        *frame;
-    njs_function_t     *function;
-    const njs_value_t  *this;
+    njs_int_t           ret;
+    njs_function_t      *function;
+    const njs_value_t   *this;
+    njs_native_frame_t  *frame;
 
     if (!njs_is_function(&args[0])) {
         njs_type_error(vm, "\"this\" argument is not a function");
@@ -1054,12 +1052,12 @@ njs_function_prototype_call(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
         nargs = 0;
     }
 
-    frame = (njs_frame_t *) vm->top_frame;
-
-    function = njs_function(&args[0]);
+    frame = vm->top_frame;
 
     /* Skip the "call" method frame. */
-    vm->top_frame->skip = 1;
+    frame->skip = 1;
+
+    function = njs_function(&args[0]);
 
     ret = njs_function_frame(vm, function, this, &args[2], nargs, 0);
     if (njs_slow_path(ret != NJS_OK)) {
@@ -1144,7 +1142,7 @@ activate:
         return ret;
     }
 
-    ret = njs_function_frame_invoke(vm, frame->retval);
+    ret = njs_function_frame_invoke(vm, frame->native.retval);
     if (njs_slow_path(ret != NJS_OK)) {
         return ret;
     }

diff --git a/src/njs_function.h b/src/njs_function.h
index 7724277fe106683843ca85ac458618b01e1865fa..159f47cec505e26ef8790184992732418fce749c 100644 (file)
--- a/src/njs_function.h
+++ b/src/njs_function.h
@@ -39,10 +39,6 @@ struct njs_function_lambda_s {
     njs_align_size(sizeof(njs_frame_t) + closures * sizeof(njs_closure_t *),  \
                    sizeof(njs_value_t))
 
-/* The retval field is not used in the global frame. */
-#define NJS_GLOBAL_FRAME_SIZE                                                 \
-    njs_align_size(offsetof(njs_frame_t, retval), sizeof(njs_value_t))
-
 #define NJS_FRAME_SPARE_SIZE       512
 
 
@@ -68,6 +64,7 @@ struct njs_native_frame_s {
     njs_object_t                   *arguments_object;
 
     njs_exception_t                exception;
+    njs_index_t                    retval;
 
     uint32_t                       size;
     uint32_t                       free_size;
@@ -84,9 +81,6 @@ struct njs_native_frame_s {
 struct njs_frame_s {
     njs_native_frame_t             native;
 
-    njs_index_t                    retval;
-
-    u_char                         *return_address;
     njs_frame_t                    *previous_active_frame;
 
     njs_value_t                    *local;
@@ -170,13 +164,12 @@ njs_function_previous_frame(njs_native_frame_t *frame)
 njs_inline njs_int_t
 njs_function_frame_invoke(njs_vm_t *vm, njs_index_t retval)
 {
-    njs_frame_t  *frame;
-
-    frame = (njs_frame_t *) vm->top_frame;
+    njs_native_frame_t  *frame;
 
+    frame = vm->top_frame;
     frame->retval = retval;
 
-    if (frame->native.function->native) {
+    if (frame->function->native) {
         return njs_function_native_call(vm);
 
     } else {

diff --git a/src/njs_promise.c b/src/njs_promise.c
index cf0a879e5904f337c61534f9b874379c9f7a7001..c1db7bcfff93ad5d78490966614f01db3cf8041c 100644 (file)
--- a/src/njs_promise.c
+++ b/src/njs_promise.c
@@ -559,16 +559,16 @@ njs_promise_resolve_function(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     njs_index_t unused)
 {
     njs_int_t              ret;
-    njs_frame_t            *active_frame;
     njs_value_t            *resolution, error, then, arguments[3];
     njs_promise_t          *promise;
     njs_function_t         *function;
+    njs_native_frame_t     *active_frame;
     njs_promise_context_t  *context;
 
     static const njs_value_t  string_then = njs_string("then");
 
-    active_frame = (njs_frame_t *) vm->top_frame;
-    context = active_frame->native.function->context;
+    active_frame = vm->top_frame;
+    context = active_frame->function->context;
     promise = njs_promise(&context->promise);
 
     if (*context->resolved_ref) {
@@ -715,12 +715,12 @@ static njs_int_t
 njs_promise_reject_function(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs,
     njs_index_t unused)
 {
-    njs_frame_t            *active_frame;
     njs_value_t            *value;
+    njs_native_frame_t     *active_frame;
     njs_promise_context_t  *context;
 
-    active_frame = (njs_frame_t *) vm->top_frame;
-    context = active_frame->native.function->context;
+    active_frame = vm->top_frame;
+    context = active_frame->function->context;
 
     if (*context->resolved_ref) {
         njs_vm_retval_set(vm, &njs_value_undefined);
@@ -995,12 +995,12 @@ njs_promise_then_finally_function(njs_vm_t *vm, njs_value_t *args,
 {
     njs_int_t              ret;
     njs_value_t            value, retval;
-    njs_frame_t            *frame;
     njs_promise_t          *promise;
+    njs_native_frame_t     *frame;
     njs_promise_context_t  *context;
 
-    frame = (njs_frame_t *) vm->top_frame;
-    context = frame->native.function->context;
+    frame = vm->top_frame;
+    context = frame->function->context;
 
     ret = njs_function_call(vm, njs_function(&context->finally),
                             &njs_value_undefined, args, 0, &retval);

diff --git a/src/njs_vm.c b/src/njs_vm.c
index 15acd5d716d00d735826fdcb8b5c8a97f826cdae..2dffb7d9ec26cf13a6faa87e904e6f4956835850 100644 (file)
--- a/src/njs_vm.c
+++ b/src/njs_vm.c
@@ -286,7 +286,7 @@ njs_vm_init(njs_vm_t *vm)
 
     scope_size = vm->scope_size + NJS_INDEX_GLOBAL_OFFSET;
 
-    size = NJS_GLOBAL_FRAME_SIZE + scope_size + NJS_FRAME_SPARE_SIZE;
+    size = njs_frame_size(0) + scope_size + NJS_FRAME_SPARE_SIZE;
     size = njs_align_size(size, NJS_FRAME_SPARE_SIZE);
 
     frame = njs_mp_align(vm->mem_pool, sizeof(njs_value_t), size);
@@ -294,15 +294,15 @@ njs_vm_init(njs_vm_t *vm)
         return NJS_ERROR;
     }
 
-    njs_memzero(frame, NJS_GLOBAL_FRAME_SIZE);
+    njs_memzero(frame, njs_frame_size(0));
 
     vm->top_frame = &frame->native;
     vm->active_frame = frame;
 
     frame->native.size = size;
-    frame->native.free_size = size - (NJS_GLOBAL_FRAME_SIZE + scope_size);
+    frame->native.free_size = size - (njs_frame_size(0) + scope_size);
 
-    values = (u_char *) frame + NJS_GLOBAL_FRAME_SIZE;
+    values = (u_char *) frame + njs_frame_size(0);
 
     frame->native.free = values + scope_size;
 
@@ -357,11 +357,12 @@ njs_vm_invoke(njs_vm_t *vm, njs_function_t *function, const njs_value_t *args,
 
 
 void
-njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame,
+njs_vm_scopes_restore(njs_vm_t *vm, njs_native_frame_t *native,
     njs_native_frame_t *previous)
 {
     njs_uint_t      n, nesting;
     njs_value_t     *args;
+    njs_frame_t     *frame;
     njs_closure_t   **closures;
     njs_function_t  *function;
 
@@ -376,7 +377,7 @@ njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame,
 
     vm->scopes[NJS_SCOPE_CALLEE_ARGUMENTS] = args;
 
-    function = frame->native.function;
+    function = native->function;
 
     if (function->native) {
         return;
@@ -386,6 +387,7 @@ njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame,
         /* GC: release function closures. */
     }
 
+    frame = (njs_frame_t *) native;
     frame = frame->previous_active_frame;
     vm->active_frame = frame;
 

diff --git a/src/njs_vm.h b/src/njs_vm.h
index 48c8fea83627f60e4ded1be9d539772c4e027d41..fabe159b664bee6763f9f4461fec906bc10a3e0e 100644 (file)
--- a/src/njs_vm.h
+++ b/src/njs_vm.h
@@ -286,7 +286,7 @@ struct njs_vm_shared_s {
 };
 
 
-void njs_vm_scopes_restore(njs_vm_t *vm, njs_frame_t *frame,
+void njs_vm_scopes_restore(njs_vm_t *vm, njs_native_frame_t *frame,
     njs_native_frame_t *previous);
 njs_int_t njs_vm_add_backtrace_entry(njs_vm_t *vm, njs_arr_t *stack,
     njs_native_frame_t *native_frame);

diff --git a/src/njs_vmcode.c b/src/njs_vmcode.c
index 6b74c4aabef8076a37e59c43584e967b0167f838..6707854392946dd66643b0c1f9510754b40ec064 100644 (file)
--- a/src/njs_vmcode.c
+++ b/src/njs_vmcode.c
@@ -705,13 +705,13 @@ next:
 
                 previous = njs_function_previous_frame(&frame->native);
 
-                njs_vm_scopes_restore(vm, frame, previous);
+                njs_vm_scopes_restore(vm, &frame->native, previous);
 
                 /*
                  * If a retval is in a callee arguments scope it
                  * must be in the previous callee arguments scope.
                  */
-                retval = njs_vmcode_operand(vm, frame->retval);
+                retval = njs_vmcode_operand(vm, frame->native.retval);
 
                 /*
                  * GC: value external/internal++ depending on
@@ -923,7 +923,7 @@ error:
 
         lambda_call = (frame == vm->active_frame);
 
-        njs_vm_scopes_restore(vm, frame, previous);
+        njs_vm_scopes_restore(vm, &frame->native, previous);
 
         if (frame->native.size != 0) {
             vm->stack_size -= frame->native.size;
@@ -1687,13 +1687,13 @@ njs_vmcode_return(njs_vm_t *vm, njs_value_t *invld, njs_value_t *retval)
 
     previous = njs_function_previous_frame(&frame->native);
 
-    njs_vm_scopes_restore(vm, frame, previous);
+    njs_vm_scopes_restore(vm, &frame->native, previous);
 
     /*
      * If a retval is in a callee arguments scope it
      * must be in the previous callee arguments scope.
      */
-    retval = njs_vmcode_operand(vm, frame->retval);
+    retval = njs_vmcode_operand(vm, frame->native.retval);
 
     /* GC: value external/internal++ depending on value and retval type */
     *retval = *value;