From 7d5d3b930648483e79cfb911f4843d2215d1058d Mon Sep 17 00:00:00 2001
From: Kaz Kylheku <kaz@kylheku.com>
Date: Sat, 18 Mar 2017 08:39:34 -0700
Subject: Restore package and package alist in handlers.

When setting up a handler frame, we note down the current
package alist and package in the frame. Then when invoking the
handler, we rebind the *package* and *package-alist* special
variables.

This is a needed security measure for sandboxing.  Since
handlers do not unwind (and therefore do not restore special
variables) a handler in sandboxed code could catch an
exception from non-sandboxed code that has changed *package*
or *package-alist*, and take advantage of those changed
values to escape from the sandbox.

* unwind.c (uw_push_handler): Store current package and
package-alist into new fields in the handler frame.
(invoke_handler): Set up a new dynamic environment and
bind *package* and *package-alist* around the handler
call, to the values noted in the frame. Thus the handler
executes with whatever package context was current when
the handler was established.

* unwind.h (struct uw_handler): New members, package and
package_alist.

* txr.1: Add paragraph to Exception Handling about this
issue.
---
 txr.1    | 28 ++++++++++++++++++++++++++++
 unwind.c | 10 ++++++++++
 unwind.h |  1 +
 3 files changed, 39 insertions(+)

diff --git a/txr.1 b/txr.1
index 012d9686..85f47e25 100644
--- a/txr.1
+++ b/txr.1
@@ -33724,6 +33724,34 @@ to the outermost. When an eligible handle is encountered, it is called. If
 it returns, the search continues. When an eligible catch is encountered,
 the search stops and a control transfer takes place to the catch site.
 
+.NP* Handlers and Sandboxing
+
+Because handlers execute in the dynamic context of the exception origin,
+without any unwinding having taken place, they expose a potential route
+of sandbox escape via the package system, unless special steps are taken.
+The threat is that code at the handler site could take advantage of
+the current value of the
+.code *package*
+and
+.code *package-list*
+variables established at the exception throw site to gain inappropriate access
+to symbols.
+
+For this reason, when a handler is established, the current values of
+.code *package*
+and
+.code *package-list*
+are recorded into the handler frame.
+When that handler is later invoked, it executes in a dynamic environment
+in which those variables are bound to the previously noted values.
+
+The catch mechanism doesn't do any such thing because the unwinding
+which is performed prior to the invocation of a catch implicitly
+restores the values of
+.B all
+special variables to the values they had at the time the frame was
+established.
+
 .NP* Exception Type Hierarchy
 
 Exception type symbols are arranged
diff --git a/unwind.c b/unwind.c
index dc883396..4d96b875 100644
--- a/unwind.c
+++ b/unwind.c
@@ -502,6 +502,8 @@ void uw_push_handler(uw_frame_t *fr, val matches, val fun)
   fr->ha.fun = fun;
   fr->ha.visible = 1;
   fr->ha.up = uw_stack;
+  fr->ha.package = cur_package;
+  fr->ha.package_alist = deref(cur_package_alist_loc);
   uw_stack = fr;
 }
 
@@ -519,14 +521,22 @@ val uw_exception_subtype_p(val sub, val sup)
 
 static void invoke_handler(uw_frame_t *fr, struct args *args)
 {
+  val saved_dyn_env = dyn_env;
+
   fr->ha.visible = 0;
 
   uw_simple_catch_begin;
 
+  dyn_env = make_env(nil, nil, dyn_env);
+
+  env_vbind(dyn_env, package_s, fr->ha.package);
+  env_vbind(dyn_env, package_alist_s, fr->ha.package_alist);
+
   generic_funcall(fr->ha.fun, args);
 
   uw_unwind {
     fr->ha.visible = 1;
+    dyn_env = saved_dyn_env;
   }
 
   uw_catch_end;
diff --git a/unwind.h b/unwind.h
index 7f8cec02..c887a423 100644
--- a/unwind.h
+++ b/unwind.h
@@ -70,6 +70,7 @@ struct uw_handler {
   val matches; /* Same position as in uw_catch! */
   int visible; /* Likewise. */
   val fun;
+  val package, package_alist;
 };
 
 struct uw_cont_copy {
-- 
cgit v1.2.3