Browse Source

HDFS-4401. Fix bug in DomainSocket path validation. Contributed by Colin Patrick McCabe.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/HDFS-347@1433229 13f79535-47bb-0310-9956-ffa450edef68
Todd Lipcon 12 years ago
parent
commit
35a145d92f

+ 16 - 11
hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c

@@ -232,7 +232,7 @@ Java_org_apache_hadoop_net_unix_DomainSocket_validateSocketPathSecurity0(
 JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents)
 JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents)
 {
 {
   jint utfLength;
   jint utfLength;
-  char path[PATH_MAX], check[PATH_MAX] = { 0 }, *token, *rest;
+  char path[PATH_MAX], check[PATH_MAX], *token, *rest;
   struct stat st;
   struct stat st;
   int ret, mode, strlenPath;
   int ret, mode, strlenPath;
   uid_t uid;
   uid_t uid;
@@ -263,21 +263,26 @@ JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents)
           "must not end in a slash.", path);
           "must not end in a slash.", path);
     goto done;
     goto done;
   }
   }
-  rest = path;
-  while ((token = strtok_r(rest, "/", &rest))) {
-    strcat(check, "/");
+  // This loop iterates through all of the path components except for the very
+  // last one.  We don't validate the last component, since it's not supposed to
+  // be a directory.  (If it is a directory, we will fail to create the socket
+  // later with EISDIR or similar.)
+  for (check[0] = '/', check[1] = '\0', rest = path, token = "";
+       token && rest[0];
+       token = strtok_r(rest, "/", &rest)) {
+    if (strcmp(check, "/") != 0) {
+      // If the previous directory we checked was '/', we skip appending another
+      // slash to the end because it would be unncessary.  Otherwise we do it.
+      strcat(check, "/");
+    }
+    // These strcats are safe because the length of 'check' is the same as the
+    // length of 'path' and we never add more slashes than were in the original
+    // path.
     strcat(check, token);
     strcat(check, token);
     if (skipComponents > 0) {
     if (skipComponents > 0) {
       skipComponents--;
       skipComponents--;
       continue;
       continue;
     }
     }
-    if (!index(rest, '/')) {
-      /* Don't validate the last component, since it's not supposed to be a
-       * directory.  (If it is a directory, we will fail to create the socket
-       * later with EISDIR or similar.)
-       */
-      break;
-    }
     if (stat(check, &st) < 0) {
     if (stat(check, &st) < 0) {
       ret = errno;
       ret = errno;
       jthr = newIOException(env, "failed to stat a path component: '%s'.  "
       jthr = newIOException(env, "failed to stat a path component: '%s'.  "

+ 1 - 1
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java

@@ -698,7 +698,7 @@ public class TestDomainSocket {
             "component: ", e);
             "component: ", e);
       }
       }
       // Root should be secure
       // Root should be secure
-      DomainSocket.validateSocketPathSecurity0("/foo", 0);
+      DomainSocket.validateSocketPathSecurity0("/foo", 1);
     } finally {
     } finally {
       tmp.close();
       tmp.close();
     }
     }

+ 3 - 0
hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-347.txt

@@ -19,3 +19,6 @@ HDFS-4390. Bypass UNIX domain socket unit tests when they cannot be run.
 
 
 HDFS-4400. DFSInputStream#getBlockReader: last retries should ignore the cache
 HDFS-4400. DFSInputStream#getBlockReader: last retries should ignore the cache
 (Colin Patrick McCabe via todd)
 (Colin Patrick McCabe via todd)
+
+HDFS-4401. Fix bug in DomainSocket path validation
+(Colin Patrick McCabe via todd)