Drop 'fr' parameter from sandbox code.

Appearently, the majority of the filenames we pass to
sandbox_cfg_allow() functions are "freeable right after". So, consider
_all_ of them safe-to-steal, and add a tor_strdup() in the few cases
that aren't.

(Maybe buggy; revise when I can test.)

2 files changed, 38 insertions, 70 deletions

diff --git a/src/common/sandbox.c b/src/common/sandbox.c
index c2f482d0c..b97b900c8 100644
--- a/src/common/sandbox.c
+++ b/src/common/sandbox.c
@@ -1056,11 +1056,11 @@ new_element(int syscall, int index, intptr_t value)
 #endif
 
 int
-sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file, int fr)
+sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_stat, 0, (intptr_t)(void*) tor_strdup(file));
+  elem = new_element(SCMP_stat, 0, (intptr_t)(void*) file);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
@@ -1069,7 +1069,6 @@ sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file, int fr)
   elem->next = *cfg;
   *cfg = elem;
 
-  if (fr) tor_free(file);
   return 0;
 }
 
@@ -1083,9 +1082,7 @@ sandbox_cfg_allow_stat_filename_array(sandbox_cfg_t **cfg, ...)
   va_start(ap, cfg);
 
   while ((fn = va_arg(ap, char*)) != NULL) {
-    int fr = va_arg(ap, int);
-
-    rc = sandbox_cfg_allow_stat_filename(cfg, fn, fr);
+    rc = sandbox_cfg_allow_stat_filename(cfg, fn);
     if (rc) {
       log_err(LD_BUG,"(Sandbox) sandbox_cfg_allow_stat_filename_array fail");
       goto end;
@@ -1098,11 +1095,11 @@ sandbox_cfg_allow_stat_filename_array(sandbox_cfg_t **cfg, ...)
 }
 
 int
-sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, int fr)
+sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_SYS(open), 0, (intptr_t)(void *)tor_strdup(file));
+  elem = new_element(SCMP_SYS(open), 0, (intptr_t)(void *) file);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
@@ -1111,8 +1108,6 @@ sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file, int fr)
   elem->next = *cfg;
   *cfg = elem;
 
-  if (fr) tor_free(file);
-
   return 0;
 }
 
@@ -1122,8 +1117,8 @@ sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2)
   sandbox_cfg_t *elem = NULL;
 
   elem = new_element2(SCMP_SYS(rename), 0, 1,
-                      (intptr_t)(void *)tor_strdup(file1),
-                      (intptr_t)(void *)tor_strdup(file2));
+                      (intptr_t)(void *) file1,
+                      (intptr_t)(void *) file2);
 
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
@@ -1142,8 +1137,6 @@ sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2)
   elem->next = *cfg;
   *cfg = elem;
 
-  tor_free(file1);
-  tor_free(file2);
   return 0;
 }
 
@@ -1157,9 +1150,7 @@ sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...)
   va_start(ap, cfg);
 
   while ((fn = va_arg(ap, char*)) != NULL) {
-    int fr = va_arg(ap, int);
-
-    rc = sandbox_cfg_allow_open_filename(cfg, fn, fr);
+    rc = sandbox_cfg_allow_open_filename(cfg, fn);
     if (rc) {
       log_err(LD_BUG,"(Sandbox) sandbox_cfg_allow_open_filename_array fail");
       goto end;
@@ -1172,11 +1163,11 @@ sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...)
 }
 
 int
-sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file, int fr)
+sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_SYS(openat), 1, (intptr_t)(void *)tor_strdup(file));
+  elem = new_element(SCMP_SYS(openat), 1, (intptr_t)(void *) file);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
@@ -1185,8 +1176,6 @@ sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file, int fr)
   elem->next = *cfg;
   *cfg = elem;
 
-  if (fr) tor_free(file);
-
   return 0;
 }
 
@@ -1200,9 +1189,7 @@ sandbox_cfg_allow_openat_filename_array(sandbox_cfg_t **cfg, ...)
   va_start(ap, cfg);
 
   while ((fn = va_arg(ap, char*)) != NULL) {
-    int fr = va_arg(ap, int);
-
-    rc = sandbox_cfg_allow_openat_filename(cfg, fn, fr);
+    rc = sandbox_cfg_allow_openat_filename(cfg, fn);
     if (rc) {
       log_err(LD_BUG,"(Sandbox) sandbox_cfg_allow_openat_filename_array fail");
       goto end;
@@ -1219,7 +1206,7 @@ sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com)
 {
   sandbox_cfg_t *elem = NULL;
 
-  elem = new_element(SCMP_SYS(execve), 1, (intptr_t)(void *)tor_strdup(com));
+  elem = new_element(SCMP_SYS(execve), 1, (intptr_t)(void *) com);
   if (!elem) {
     log_err(LD_BUG,"(Sandbox) failed to register parameter!");
     return -1;
@@ -1519,7 +1506,8 @@ register_cfg(sandbox_cfg_t* cfg)
     return 0;
   }
 
-  for (elem = filter_dynamic; elem->next != NULL; elem = elem->next);
+  for (elem = filter_dynamic; elem->next != NULL; elem = elem->next)
+    ;
 
   elem->next = cfg;
 
@@ -1583,10 +1571,9 @@ sandbox_init(sandbox_cfg_t *cfg)
 
 #ifndef USE_LIBSECCOMP
 int
-sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file,
-                                int fr)
+sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file)
 {
-  (void)cfg; (void)file; (void)fr;
+  (void)cfg; (void)file;
   return 0;
 }
 
@@ -1598,10 +1585,9 @@ sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...)
 }
 
 int
-sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file,
-                                  int fr)
+sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file)
 {
-  (void)cfg; (void)file; (void)fr;
+  (void)cfg; (void)file;
   return 0;
 }
 
@@ -1627,10 +1613,9 @@ sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...)
 }
 
 int
-sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file,
-                                int fr)
+sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file)
 {
-  (void)cfg; (void)file; (void)fr;
+  (void)cfg; (void)file;
   return 0;
 }
 
diff --git a/src/common/sandbox.h b/src/common/sandbox.h
index b15f31cc4..8fcd221ef 100644
--- a/src/common/sandbox.h
+++ b/src/common/sandbox.h
@@ -171,12 +171,10 @@ sandbox_cfg_t * sandbox_cfg_new(void);
 
 /**
  * Function used to add a open allowed filename to a supplied configuration.
- * The (char*) specifies the path to the allowed file, fr = 1 tells the
- * function that the char* needs to be free-ed, 0 means the pointer does not
- * need to be free-ed.
+ * The (char*) specifies the path to the allowed file; we take ownership
+ * of the pointer.
  */
-int sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file,
-    int fr);
+int sandbox_cfg_allow_open_filename(sandbox_cfg_t **cfg, char *file);
 
 /**DOCDOC*/
 int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2);
@@ -184,66 +182,51 @@ int sandbox_cfg_allow_rename(sandbox_cfg_t **cfg, char *file1, char *file2);
 /** Function used to add a series of open allowed filenames to a supplied
  * configuration.
  *  @param cfg  sandbox configuration.
- *  @param ... all future parameters are specified as pairs of <(char*), 1 / 0>
- *    the char* specifies the path to the allowed file, 1 tells the function
- *    that the char* needs to be free-ed, 0 means the pointer does not need to
- *    be free-ed; the final parameter needs to be <NULL, 0>.
- */
+ *  @param ... a list of stealable pointers to permitted files.  The last
+ *  one must be NULL.
+*/
 int sandbox_cfg_allow_open_filename_array(sandbox_cfg_t **cfg, ...);
 
 /**
  * Function used to add a openat allowed filename to a supplied configuration.
- * The (char*) specifies the path to the allowed file, fr = 1 tells the
- * function that the char* needs to be free-ed, 0 means the pointer does not
- * need to be free-ed.
+ * The (char*) specifies the path to the allowed file; we steal the pointer to
+ * that file.
  */
-int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file,
-    int fr);
+int sandbox_cfg_allow_openat_filename(sandbox_cfg_t **cfg, char *file);
 
 /** Function used to add a series of openat allowed filenames to a supplied
  * configuration.
  *  @param cfg  sandbox configuration.
- *  @param ... all future parameters are specified as pairs of <(char*), 1 / 0>
- *    the char* specifies the path to the allowed file, 1 tells the function
- *    that the char* needs to be free-ed, 0 means the pointer does not need to
- *    be free-ed; the final parameter needs to be <NULL, 0>.
+ *  @param ... a list of stealable pointers to permitted files.  The last
+ *  one must be NULL.
  */
 int sandbox_cfg_allow_openat_filename_array(sandbox_cfg_t **cfg, ...);
 
 /**
  * Function used to add a execve allowed filename to a supplied configuration.
- * The (char*) specifies the path to the allowed file, fr = 1 tells the
- * function that the char* needs to be free-ed, 0 means the pointer does not
- * need to be free-ed.
+ * The (char*) specifies the path to the allowed file; that pointer is stolen.
  */
 int sandbox_cfg_allow_execve(sandbox_cfg_t **cfg, const char *com);
 
 /** Function used to add a series of execve allowed filenames to a supplied
  * configuration.
  *  @param cfg  sandbox configuration.
- *  @param ... all future parameters are specified as pairs of <(char*), 1 / 0>
- *    the char* specifies the path to the allowed file, 1 tells the function
- *    that the char* needs to be free-ed, 0 means the pointer does not need to
- *    be free-ed; the final parameter needs to be <NULL, 0>.
+ *  @param ... an array of stealable pointers to permitted files.  The last
+ *  one must be NULL.
  */
 int sandbox_cfg_allow_execve_array(sandbox_cfg_t **cfg, ...);
 
 /**
  * Function used to add a stat/stat64 allowed filename to a configuration.
- * The (char*) specifies the path to the allowed file, fr = 1 tells the
- * function that the char* needs to be free-ed, 0 means the pointer does not
- * need to be free-ed.
+ * The (char*) specifies the path to the allowed file; that pointer is stolen.
  */
-int sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file,
-    int fr);
+int sandbox_cfg_allow_stat_filename(sandbox_cfg_t **cfg, char *file);
 
 /** Function used to add a series of stat64 allowed filenames to a supplied
  * configuration.
  *  @param cfg  sandbox configuration.
- *  @param ... all future parameters are specified as pairs of <(char*), 1 / 0>
- *    the char* specifies the path to the allowed file, 1 tells the function
- *    that the char* needs to be free-ed, 0 means the pointer does not need to
- *    be free-ed; the final parameter needs to be <NULL, 0>.
+ *  @param ... an array of stealable pointers to permitted files.  The last
+ *  one must be NULL.
  */
 int sandbox_cfg_allow_stat_filename_array(sandbox_cfg_t **cfg, ...);