[tor-commits] [tor/master] When handling includes, detect missing interned strings earlier.

[asn at torproject.org]

commit dbc8d2a4e476c06f59db3ff79b66afc8bc4ea27c
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Nov 12 11:55:55 2020 -0500

    When handling includes, detect missing interned strings earlier.
    
    There were three separate places where we were hitting a sandbox Bug
    warning before we actually exited.
    
    Fixes #40094; bugfix on 0.3.1.1-alpha when %includes were introduced.
---
 src/lib/fs/conffile.c     | 27 +++++++++++++++++++++------
 src/lib/sandbox/sandbox.c | 37 ++++++++++++++++++++++++++++++++++---
 src/lib/sandbox/sandbox.h |  5 ++---
 3 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/src/lib/fs/conffile.c b/src/lib/fs/conffile.c
index f1f6d8ae5f..1f58a3590c 100644
--- a/src/lib/fs/conffile.c
+++ b/src/lib/fs/conffile.c
@@ -19,6 +19,7 @@
 #include "lib/fs/path.h"
 #include "lib/log/log.h"
 #include "lib/malloc/malloc.h"
+#include "lib/sandbox/sandbox.h"
 #include "lib/string/printf.h"
 
 #include <stdbool.h>
@@ -59,14 +60,14 @@ config_get_lines_include(const char *string, config_line_t **result,
 static smartlist_t *
 expand_glob(const char *pattern, smartlist_t *opened_files)
 {
-  smartlist_t *matches = tor_glob(pattern);
-  if (!matches) {
-    return NULL;
+  if (! has_glob(pattern)) {
+    smartlist_t *matches = smartlist_new();
+    smartlist_add_strdup(matches, pattern);
+    return matches;
   }
 
-  // if it is not a glob, return error when the path is missing
-  if (!has_glob(pattern) && smartlist_len(matches) == 0) {
-    smartlist_free(matches);
+  smartlist_t *matches = tor_glob(pattern);
+  if (!matches) {
     return NULL;
   }
 
@@ -107,6 +108,13 @@ config_get_file_list(const char *pattern, smartlist_t *opened_files)
     if (opened_files) {
       smartlist_add_strdup(opened_files, path);
     }
+    if (sandbox_interned_string_is_missing(path)) {
+      log_err(LD_CONFIG, "Sandbox is active, but a new configuration "
+              "file \"%s\" has been listed with %%include. Cannot proceed.",
+              path);
+      error_found = true;
+      break;
+    }
 
     file_status_t file_type = file_status(path);
     if (file_type == FN_FILE) {
@@ -201,6 +209,13 @@ config_process_include(const char *pattern, int recursion_level, int extended,
 
   int rv = -1;
   SMARTLIST_FOREACH_BEGIN(config_files, const char *, config_file) {
+    if (sandbox_interned_string_is_missing(config_file)) {
+      log_err(LD_CONFIG, "Sandbox is active, but a new configuration "
+              "file \"%s\" has been listed with %%include. Cannot proceed.",
+              config_file);
+      goto done;
+    }
+
     log_notice(LD_CONFIG, "Including configuration file \"%s\".", config_file);
     config_line_t *included_config = NULL;
     config_line_t *included_config_last = NULL;
diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c
index 8d467c516e..d9ad8ec2c6 100644
--- a/src/lib/sandbox/sandbox.c
+++ b/src/lib/sandbox/sandbox.c
@@ -310,6 +310,8 @@ static int filter_nopar_gen[] = {
 #define seccomp_rule_add_4(ctx,act,call,f1,f2,f3,f4)      \
   seccomp_rule_add((ctx),(act),(call),4,(f1),(f2),(f3),(f4))
 
+static const char *sandbox_get_interned_string(const char *str);
+
 /**
  * Function responsible for setting up the rt_sigaction syscall for
  * the seccomp filter sandbox.
@@ -1224,8 +1226,39 @@ static sandbox_filter_func_t filter_func[] = {
     sb_kill
 };
 
+/**
+ * Return the interned (and hopefully sandbox-permitted) string equal
+ * to @a str.
+ */
 const char *
 sandbox_intern_string(const char *str)
+{
+  const char *interned = sandbox_get_interned_string(str);
+
+  if (sandbox_active && str != NULL && interned == NULL) {
+    log_warn(LD_BUG, "No interned sandbox parameter found for %s", str);
+  }
+
+  return interned ? interned : str;
+}
+
+/**
+ * Return true if the sandbox is running and we are missing an interned string
+ * equal to @a str.
+ */
+bool
+sandbox_interned_string_is_missing(const char *str)
+{
+  return sandbox_active && sandbox_get_interned_string(str) == NULL;
+}
+
+/**
+ * Try to find and return the interned string equal to @a str.
+ *
+ * If there is no such string, return NULL.
+ **/
+static const char *
+sandbox_get_interned_string(const char *str)
 {
   sandbox_cfg_t *elem;
 
@@ -1245,9 +1278,7 @@ sandbox_intern_string(const char *str)
     }
   }
 
-  if (sandbox_active)
-    log_warn(LD_BUG, "No interned sandbox parameter found for %s", str);
-  return str;
+  return NULL;
 }
 
 /* DOCDOC */
diff --git a/src/lib/sandbox/sandbox.h b/src/lib/sandbox/sandbox.h
index a2b3227b90..eba99afbde 100644
--- a/src/lib/sandbox/sandbox.h
+++ b/src/lib/sandbox/sandbox.h
@@ -104,12 +104,11 @@ typedef struct {
 #endif /* defined(USE_LIBSECCOMP) */
 
 #ifdef USE_LIBSECCOMP
-/** Returns a registered protected string used with the sandbox, given that
- * it matches the parameter.
- */
 const char* sandbox_intern_string(const char *param);
+bool sandbox_interned_string_is_missing(const char *s);
 #else /* !defined(USE_LIBSECCOMP) */
 #define sandbox_intern_string(s) (s)
+#define sandbox_interned_string_is_missing(s) (false)
 #endif /* defined(USE_LIBSECCOMP) */
 
 /** Creates an empty sandbox configuration file.*/