[patch] sandbox-PATH_MAX-fix.patch

sandbox-PATH_MAX-fix.patch (text/plain), 12.22 KB, created by Martin Schlemmer (RETIRED) on 2003-06-22 12:43:51 UTC

Index: sandbox-1.1/ChangeLog
===================================================================
RCS file: /home/cvsroot/gentoo-src/portage/src/sandbox-1.1/ChangeLog,v
retrieving revision 1.15
diff -u -r1.15 ChangeLog
--- sandbox-1.1/ChangeLog	6 Apr 2003 09:06:48 -0000	1.15
+++ sandbox-1.1/ChangeLog	22 Jun 2003 19:36:38 -0000
@@ -2,6 +2,34 @@
 # Copyright 2002 Gentoo Technologies, Inc.; Distributed under the GPL v2
 # $Header: /home/cvsroot/gentoo-src/portage/src/sandbox-1.1/ChangeLog,v 1.15 2003/04/06 09:06:48 azarah Exp $
 
+  22 Jun 2003; Martin Schlemmer <[e-mail]> libsandbox.c, canonicalize.c
+  create-localdecls :
+  When checking path names of files accessed, we need to canonicalize it, else
+  it may be a symlink in a 'write allowed' directory pointing to a file in a
+  directory we should not have write access to.
+
+  With something like coreutils-5.0, we have two problems:
+  1)  One of the tests checks if getcwd() can return a path longer than
+      PATH_MAX.  This test then tries to create a dir which even while
+      created local (mkdir("conftest2")), it ends up being resolved with
+      a name that is much larger than PATH_MAX.  The problem now is that
+      canonicalize() have undefined behaviour when the path was too long
+      (returned wrongly truncated paths, etc), and pass the wrong path to
+      before_syscall() (causing the bogus sandbox violations).
+  2)  The ecanonicalize() function we used, along with the canonicalize()
+      function did not support longer than PATH_MAX.  This is partly a
+      cause for 1), but the error checking (rather lack of it) of calls
+      to erealpath() in canonicalize() was the prime reason for 1).
+
+  As we do not use this canonicalized name to call the function, we resolve this
+  by fixing canonicalize() to do better error checking, and ecanonicalize() as
+  well as all functions in libsandbox.c to use a PATH_MAX of 'PATH_MAX * 2'.
+  While they will resolve paths properly now, and can check if a write/read is
+  allowed, the functions called from the sandboxed environment will still work
+  as expected.
+
+  This should resolve bug #21766.
+
   06 Apr 2003; Martin Schlemmer <[e-mail]> libsandbox.c :
   For some reason sandbox fails with a 'open_wr' if you run 'locale -a' under
   it (bug #16298).
Index: sandbox-1.1/Makefile
===================================================================
RCS file: /home/cvsroot/gentoo-src/portage/src/sandbox-1.1/Makefile,v
retrieving revision 1.3
diff -u -r1.3 Makefile
--- sandbox-1.1/Makefile	22 Feb 2003 13:40:38 -0000	1.3
+++ sandbox-1.1/Makefile	22 Jun 2003 19:36:38 -0000
@@ -37,7 +37,7 @@
 libsandbox.o: libsandbox.c localdecls.h
 	$(CC) $(CFLAGS) -Wall -c $(OBJ_DEFINES) libsandbox.c
 
-canonicalize.o: canonicalize.c
+canonicalize.o: canonicalize.c localdecls.h
 	$(CC) $(CFLAGS) -Wall -c $(OBJ_DEFINES) canonicalize.c
 
 localdecls.h: create-localdecls libctest.c
Index: sandbox-1.1/canonicalize.c
===================================================================
RCS file: /home/cvsroot/gentoo-src/portage/src/sandbox-1.1/canonicalize.c,v
retrieving revision 1.2
diff -u -r1.2 canonicalize.c
--- sandbox-1.1/canonicalize.c	26 Aug 2002 19:40:31 -0000	1.2
+++ sandbox-1.1/canonicalize.c	22 Jun 2003 19:36:38 -0000
@@ -29,6 +29,8 @@
 #include <sys/stat.h>
 #include <errno.h>
 #include <stddef.h>
+
+#include "localdecls.h"
                      
 #ifndef __set_errno
 # define __set_errno(val) errno = (val)
@@ -38,8 +40,8 @@
    does not contain any `.', `..' components nor any repeated path
    separators ('/') or symlinks.  All path components must exist.  If
    RESOLVED is null, the result is malloc'd; otherwise, if the
-   canonical name is PATH_MAX chars or more, returns null with `errno'
-   set to ENAMETOOLONG; if the name fits in fewer than PATH_MAX chars,
+   canonical name is SB_PATH_MAX chars or more, returns null with `errno'
+   set to ENAMETOOLONG; if the name fits in fewer than SB_PATH_MAX chars,
    returns the name in RESOLVED.  If the name cannot be resolved and
    RESOLVED is non-NULL, it contains the path of the first component
    that cannot be resolved.  If the path can be resolved, RESOLVED
@@ -78,8 +80,8 @@
       return NULL;
     }
 
-#ifdef PATH_MAX
-  path_max = PATH_MAX;
+#ifdef SB_PATH_MAX
+  path_max = SB_PATH_MAX;
 #else
   path_max = pathconf (name, _PC_PATH_MAX);
   if (path_max <= 0)
Index: sandbox-1.1/create-localdecls
===================================================================
RCS file: /home/cvsroot/gentoo-src/portage/src/sandbox-1.1/create-localdecls,v
retrieving revision 1.6
diff -u -r1.6 create-localdecls
--- sandbox-1.1/create-localdecls	23 Feb 2003 13:13:36 -0000	1.6
+++ sandbox-1.1/create-localdecls	22 Jun 2003 19:36:38 -0000
@@ -92,6 +92,20 @@
 	esac
 fi
 
+echo '
+#ifdef PATH_MAX
+# define SB_PATH_MAX PATH_MAX * 2
+# if (SB_PATH_MAX >= INT_MAX)
+#  undef SB_PATH_MAX
+#  define SB_PATH_MAX PATH_MAX + 25
+#  if (SB_PATH_MAX >= INT_MAX)
+#   error PATH_MAX too big!
+#  endif
+# endif
+#else
+# error PATH_MAX not defined!
+#endif' >> $OUTFILE
+
 echo >> $OUTFILE
 echo '#endif' >> $OUTFILE
 echo
Index: sandbox-1.1/libsandbox.c
===================================================================
RCS file: /home/cvsroot/gentoo-src/portage/src/sandbox-1.1/libsandbox.c,v
retrieving revision 1.4
diff -u -r1.4 libsandbox.c
--- sandbox-1.1/libsandbox.c	6 Apr 2003 09:06:48 -0000	1.4
+++ sandbox-1.1/libsandbox.c	22 Jun 2003 19:36:38 -0000
@@ -93,6 +93,18 @@
   errno=old_errno; \
 }
 
+#define canonicalize(path, resolved_path) \
+{ \
+  if (0 != _canonicalize(path, resolved_path)) \
+    return -1; \
+}
+
+#define canonicalize_ptr(path, resolved_path) \
+{ \
+  if (0 != _canonicalize(path, resolved_path)) \
+    return NULL; \
+}
+
 static char sandbox_lib[255];
 
 typedef struct {
@@ -114,7 +126,7 @@
 
 static void init_wrappers(void);
 static void *get_dlsym(const char *);
-static void canonicalize(const char *, char *);
+static int _canonicalize(const char *, char *);
 static int check_access(sbcontext_t *, const char *, const char *);
 static int check_syscall(sbcontext_t *, const char *, const char *);
 static int before_syscall(const char *, const char *);
@@ -239,25 +251,50 @@
   errno = old_errno;
 }
 
-static void canonicalize(const char *path, char *resolved_path)
+static int _canonicalize(const char *path, char *resolved_path)
 {
   int old_errno = errno;
+  char *retval;
 
   /* If path == NULL, return or we get a segfault */
-  if (NULL == path) return;
+  if (NULL == path) {
+    errno = EINVAL;
+    return -1;
+  }
+
+  retval = erealpath(path, resolved_path);
   
-  if(!erealpath(path, resolved_path) && (path[0] != '/')) {
+  if((!retval) && (path[0] != '/')) {
     /* The path could not be canonicalized, append it
      * to the current working directory if it was not
      * an absolute path
      */
-    getcwd(resolved_path, MAXPATHLEN - 2);
+    if (errno == ENAMETOOLONG)
+      return -1;
+    
+    getcwd(resolved_path, SB_PATH_MAX - 2);
     strcat(resolved_path, "/");
-    strncat(resolved_path, path, MAXPATHLEN - 1);
-    erealpath(resolved_path, resolved_path);
+    strncat(resolved_path, path, SB_PATH_MAX - 1);
+    
+    if (!erealpath(resolved_path, resolved_path)) {
+      if (errno == ENAMETOOLONG) {
+        /* The resolved path is too long for the buffer to hold */
+        return -1;
+      } else {
+        /* Whatever it resolved, is not a valid path */
+        errno = ENOENT;
+        return -1;
+      }
+    }
+    
+  } else if ((!retval) && (path[0] == '/')) {
+    /* Whatever it resolved, is not a valid path */
+    errno = ENOENT;
+    return -1;
   }
 
   errno = old_errno;
+  return 0;
 }
 
 static void *get_dlsym(const char *symname)
@@ -291,7 +328,7 @@
 int chmod(const char *path, mode_t mode)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(path, canonic);
 
@@ -306,7 +343,7 @@
 int chown(const char *path, uid_t owner, gid_t group)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(path, canonic);
 
@@ -322,7 +359,7 @@
 {
 /* Is it a system call? */
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(pathname, canonic);
 
@@ -337,9 +374,9 @@
 FILE *fopen(const char *pathname, const char *mode)
 {
   FILE *result = NULL;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
-  canonicalize(pathname, canonic);
+  canonicalize_ptr(pathname, canonic);
 
   if FUNCTION_SANDBOX_SAFE_CHAR("fopen", canonic, mode) {
     check_dlsym(fopen);
@@ -353,7 +390,7 @@
 {
 /* Linux specific? */
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(path, canonic);
 
@@ -368,7 +405,7 @@
 int link(const char *oldpath, const char *newpath)
 {
   int result = -1;
-  char old_canonic[MAXPATHLEN], new_canonic[MAXPATHLEN];
+  char old_canonic[SB_PATH_MAX], new_canonic[SB_PATH_MAX];
 
   canonicalize(oldpath, old_canonic);
   canonicalize(newpath, new_canonic);
@@ -384,7 +421,7 @@
 int mkdir(const char *pathname, mode_t mode)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(pathname, canonic);
 
@@ -399,9 +436,9 @@
 DIR *opendir(const char *name)
 {
   DIR *result = NULL;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
-  canonicalize(name, canonic);
+  canonicalize_ptr(name, canonic);
 
   if FUNCTION_SANDBOX_SAFE("opendir", canonic) {
     check_dlsym(opendir);
@@ -416,7 +453,7 @@
 int __xmknod(const char *pathname, mode_t mode, dev_t dev)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(pathname, canonic);
 
@@ -436,7 +473,7 @@
   va_list ap;
   mode_t mode = 0;
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   if (flags & O_CREAT) {
     va_start(ap, flags);
@@ -460,7 +497,7 @@
 int rename(const char *oldpath, const char *newpath)
 {
   int result = -1;
-  char old_canonic[MAXPATHLEN], new_canonic[MAXPATHLEN];
+  char old_canonic[SB_PATH_MAX], new_canonic[SB_PATH_MAX];
 
   canonicalize(oldpath, old_canonic);
   canonicalize(newpath, new_canonic);
@@ -476,7 +513,7 @@
 int rmdir(const char *pathname)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(pathname, canonic);
 
@@ -491,7 +528,7 @@
 int symlink(const char *oldpath, const char *newpath)
 {
   int result = -1;
-  char old_canonic[MAXPATHLEN], new_canonic[MAXPATHLEN];
+  char old_canonic[SB_PATH_MAX], new_canonic[SB_PATH_MAX];
 
   canonicalize(oldpath, old_canonic);
   canonicalize(newpath, new_canonic);
@@ -507,7 +544,7 @@
 int truncate(const char *path, TRUNCATE_T length)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(path, canonic);
 
@@ -522,7 +559,7 @@
 int unlink(const char *pathname)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(pathname, canonic);
 
@@ -540,7 +577,7 @@
 {
 /* Is it a system call? */
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(pathname, canonic);
 
@@ -555,9 +592,9 @@
 FILE *fopen64(const char *pathname, const char *mode)
 {
   FILE *result = NULL;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
-  canonicalize(pathname, canonic);
+  canonicalize_ptr(pathname, canonic);
 
   if FUNCTION_SANDBOX_SAFE_CHAR("fopen64", canonic, mode) {
     check_dlsym(fopen64);
@@ -573,7 +610,7 @@
   va_list ap;
   mode_t mode = 0;
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   if (flags & O_CREAT) {
     va_start(ap, flags);
@@ -594,7 +631,7 @@
 int truncate64(const char *path, __off64_t length)
 {
   int result = -1;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
 
   canonicalize(path, canonic);
 
@@ -617,7 +654,7 @@
   int old_errno = errno;
   int result = -1;
   int count = 0;
-  char canonic[MAXPATHLEN];
+  char canonic[SB_PATH_MAX];
   char *old_envp = NULL;
   char *new_envp = NULL;
 
@@ -843,9 +880,9 @@
 static char* filter_path(const char* path)
 {
   int old_errno = errno;
-  char* filtered_path = (char *)malloc(MAXPATHLEN * sizeof(char));
+  char* filtered_path = (char *)malloc(SB_PATH_MAX * sizeof(char));
 
-  canonicalize(path, filtered_path);
+  canonicalize_ptr(path, filtered_path);
 
   errno = old_errno;