Move funcenv verification to runtime.

Lazy verification makes it easier to not leave funcenvs
in an invalid state, as well as be more precise with the validation.
We needed to verify the FuncEnvs actually pointed to a stack frame if
they were of the "on-stack" variant. There was some minor checking
before, but it was not enough to prevent func envs from pointing to
memory that was off of the fiber stack, overlapping stack frames, etc.

src/core/fiber.c

@@ -218,6 +218,7 @@ int janet_fiber_funcframe(JanetFiber *fiber, JanetFunction *func) {
 static void janet_env_detach(JanetFuncEnv *env) {
     /* Check for closure environment */
     if (env) {
+        janet_env_valid(env);
         int32_t len = env->length;
         size_t s = sizeof(Janet) * (size_t) len;
         Janet *vmem = malloc(s);
@@ -244,10 +245,38 @@ static void janet_env_detach(JanetFuncEnv *env) {
     }
 }
 
+/* Validate potentially untrusted func env (unmarshalled envs are difficult to verify) */
+int janet_env_valid(JanetFuncEnv *env) {
+    if (env->offset < 0) {
+        int32_t real_offset = -(env->offset);
+        JanetFiber *fiber = env->as.fiber;
+        int32_t i = fiber->frame;
+        while (i > 0) {
+            JanetStackFrame *frame = (JanetStackFrame *)(fiber->data + i - JANET_FRAME_SIZE);
+            if (real_offset == i &&
+                    frame->env == env &&
+                    frame->func &&
+                    frame->func->def->slotcount == env->length) {
+                env->offset = real_offset;
+                return 1;
+            }
+            i = frame->prevframe;
+        }
+        /* Invalid, set to empty off-stack variant. */
+        env->offset = 0;
+        env->length = 0;
+        env->as.values = NULL;
+        return 0;
+    } else {
+        return 1;
+    }
+}
+
 /* Detach a fiber from the env if the target fiber has stopped mutating */
 void janet_env_maybe_detach(JanetFuncEnv *env) {
     /* Check for detachable closure envs */
-    if (env->offset) {
+    janet_env_valid(env);
+    if (env->offset > 0) {
         JanetFiberStatus s = janet_fiber_status(env->as.fiber);
         int isFinished = s == JANET_STATUS_DEAD ||
                          s == JANET_STATUS_ERROR ||

src/core/fiber.h

@@ -74,5 +74,6 @@ int janet_fiber_funcframe_tail(JanetFiber *fiber, JanetFunction *func);
 void janet_fiber_cframe(JanetFiber *fiber, JanetCFunction cfun);
 void janet_fiber_popframe(JanetFiber *fiber);
 void janet_env_maybe_detach(JanetFuncEnv *env);
+int janet_env_valid(JanetFuncEnv *env);
 
 #endif

src/core/gc.c

@@ -193,7 +193,7 @@ static void janet_mark_funcenv(JanetFuncEnv *env) {
     /* If closure env references a dead fiber, we can just copy out the stack frame we need so
      * we don't need to keep around the whole dead fiber. */
     janet_env_maybe_detach(env);
-    if (env->offset) {
+    if (env->offset > 0) {
         /* On stack */
         janet_mark_fiber(env->as.fiber);
     } else {

src/core/marsh.c

@@ -183,8 +183,9 @@ static void marshal_one_env(MarshalState *st, JanetFuncEnv *env, int flags) {
             return;
         }
     }
+    janet_env_valid(env);
     janet_v_push(st->seen_envs, env);
-    if (env->offset && (JANET_STATUS_ALIVE == janet_fiber_status(env->as.fiber))) {
+    if (env->offset > 0 && (JANET_STATUS_ALIVE == janet_fiber_status(env->as.fiber))) {
         pushint(st, 0);
         pushint(st, env->length);
         Janet *values = env->as.fiber->data + env->offset;
@@ -200,7 +201,7 @@ static void marshal_one_env(MarshalState *st, JanetFuncEnv *env, int flags) {
         janet_env_maybe_detach(env);
         pushint(st, env->offset);
         pushint(st, env->length);
-        if (env->offset) {
+        if (env->offset > 0) {
             /* On stack variant */
             marshal_one(st, janet_wrap_fiber(env->as.fiber), flags + 1);
         } else {
@@ -721,11 +722,8 @@ static const uint8_t *unmarshal_one_env(
             data = unmarshal_one(st, data, &fiberv, flags);
             janet_asserttype(fiberv, JANET_FIBER);
             env->as.fiber = janet_unwrap_fiber(fiberv);
-            /* Unmarshalling fiber may set values */
-            if (env->offset != 0 && env->offset != offset)
-                janet_panic("invalid funcenv offset");
-            if (env->length != 0 && env->length != length)
-                janet_panic("invalid funcenv length");
+            /* Negative offset indicates untrusted input */
+            env->offset = -offset;
         } else {
             /* Off stack variant */
             if (length == 0) {
@@ -735,10 +733,10 @@ static const uint8_t *unmarshal_one_env(
             if (!env->as.values) {
                 JANET_OUT_OF_MEMORY;
             }
+            env->offset = 0;
             for (int32_t i = 0; i < length; i++)
                 data = unmarshal_one(st, data, env->as.values + i, flags);
         }
-        env->offset = offset;
         env->length = length;
         *out = env;
     }
@@ -981,18 +979,7 @@ static const uint8_t *unmarshal_one_fiber(
         /* Check env */
         if (frameflags & JANET_STACKFRAME_HASENV) {
             frameflags &= ~JANET_STACKFRAME_HASENV;
-            int32_t offset = stack;
-            int32_t length = stacktop - stack;
-            if (length <= 0) {
-                janet_panic("invalid funcenv length");
-            }
             data = unmarshal_one_env(st, data, &env, flags + 1);
-            if (env->offset != 0 && env->offset != offset)
-                janet_panic("funcenv offset does not match fiber frame");
-            if (env->length != 0 && env->length != length)
-                janet_panic("funcenv length does not match fiber frame");
-            env->offset = offset;
-            env->length = length;
         }
 
         /* Error checking */

src/core/vm.c

@@ -824,7 +824,8 @@ static JanetSignal run_vm(JanetFiber *fiber, Janet in) {
         vm_assert(func->def->environments_length > eindex, "invalid upvalue environment");
         env = func->envs[eindex];
         vm_assert(env->length > vindex, "invalid upvalue index");
-        if (env->offset) {
+        vm_assert(janet_env_valid(env), "invalid upvalue environment");
+        if (env->offset > 0) {
             /* On stack */
             stack[A] = env->as.fiber->data[env->offset + vindex];
         } else {
@@ -841,7 +842,8 @@ static JanetSignal run_vm(JanetFiber *fiber, Janet in) {
         vm_assert(func->def->environments_length > eindex, "invalid upvalue environment");
         env = func->envs[eindex];
         vm_assert(env->length > vindex, "invalid upvalue index");
-        if (env->offset) {
+        vm_assert(janet_env_valid(env), "invalid upvalue environment");
+        if (env->offset > 0) {
             env->as.fiber->data[env->offset + vindex] = stack[A];
         } else {
             env->as.values[vindex] = stack[A];

test/suite8.janet

@@ -232,4 +232,10 @@ neldb\0\0\0\xD8\x05printG\x01\0\xDE\xDE\xDE'\x03\0marshal_tes/\x02
 (unmarshal b load-image-dict)
 (gccollect)
 
+(def v
+  (unmarshal
+    @"\xD7\xCD0\xD4000000\0\x03\x01\xCE\00\0\x01\0\0000\x03\0\0\0000000000\xCC0\0000"
+    load-image-dict))
+(gccollect)
+
 (end-suite)