From 35a145d92f6b8513924616cca2a08e569333cfb2 Mon Sep 17 00:00:00 2001 From: Todd Lipcon Date: Tue, 15 Jan 2013 00:12:47 +0000 Subject: [PATCH] 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 --- .../org/apache/hadoop/net/unix/DomainSocket.c | 27 +++++++++++-------- .../hadoop/net/unix/TestDomainSocket.java | 2 +- .../hadoop-hdfs/CHANGES.HDFS-347.txt | 3 +++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c b/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c index 2daa4866c92..d05499609f5 100644 --- a/hadoop-common-project/hadoop-common/src/main/native/src/org/apache/hadoop/net/unix/DomainSocket.c +++ b/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) { jint utfLength; - char path[PATH_MAX], check[PATH_MAX] = { 0 }, *token, *rest; + char path[PATH_MAX], check[PATH_MAX], *token, *rest; struct stat st; int ret, mode, strlenPath; uid_t uid; @@ -263,21 +263,26 @@ JNIEnv *env, jclass clazz, jobject jstr, jint skipComponents) "must not end in a slash.", path); 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); if (skipComponents > 0) { skipComponents--; 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) { ret = errno; jthr = newIOException(env, "failed to stat a path component: '%s'. " diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java index 2d31874a0d2..a997f52056a 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/net/unix/TestDomainSocket.java @@ -698,7 +698,7 @@ public class TestDomainSocket { "component: ", e); } // Root should be secure - DomainSocket.validateSocketPathSecurity0("/foo", 0); + DomainSocket.validateSocketPathSecurity0("/foo", 1); } finally { tmp.close(); } diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-347.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-347.txt index 1289d9e21a6..96afb6a9847 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.HDFS-347.txt +++ b/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 (Colin Patrick McCabe via todd) + +HDFS-4401. Fix bug in DomainSocket path validation +(Colin Patrick McCabe via todd)