[security] [dart:io] Fix current directory being in front of PATH.
This is a security improvement.
On Linux and Android, starting a process with Process.run, Process.runSync
or Process.start would first search the current directory before searching
PATH (Issue [37101][]). Operating systems other than Linux and Android
didn't have this behavior and aren't affected by this vulnerability.
Effectively this puts the current working directory in the front of PATH,
even if it wasn't in the PATH.
This change fixes that vulnerability and only searches the directories in
the PATH environment variable.
Fixes https://github.com/dart-lang/sdk/issues/37101
Change-Id: I05f3137753237f9b3ba4be4eba63ad07a75d865e
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/105582
Reviewed-by: William Hesse <whesse@google.com>
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0cd891b..1e47fd8 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,3 +1,34 @@
+## 2.3.2 - 2019-06-11
+
+This is a patch version release with a security improvement.
+
+### Security vulnerability
+
+* **Security improvement:** On Linux and Android, starting a process with
+ `Process.run`, `Process.runSync`, or `Process.start` would first search the
+ current directory before searching `PATH` (Issue [37101][]). This behavior
+ effectively put the current working directory in the front of `PATH`, even if
+ it wasn't in the `PATH`. This release changes that behavior to only searching
+ the directories in the `PATH` environment variable. Operating systems other
+ than Linux and Android didn't have this behavior and aren't affected by this
+ vulnerability.
+
+ This vulnerability could result in execution of untrusted code if a command
+ without a slash in its name was run inside an untrusted directory containing
+ an executable file with that name:
+
+ ```dart
+ Process.run("ls", workingDirectory: "/untrusted/directory")
+ ```
+
+ This would attempt to run `/untrusted/directory/ls` if it existed, even
+ though it is not in the `PATH`. It was always safe to instead use an absolute
+ path or a path containing a slash.
+
+ This vulnerability was introduced in Dart 2.0.0.
+
+[37101]: https://github.com/dart-lang/sdk/issues/37101
+
## 2.3.1 - 2019-05-21
This is a patch version release with bug fixes.
diff --git a/runtime/bin/process_android.cc b/runtime/bin/process_android.cc
index c26e462..5995e8a 100644
--- a/runtime/bin/process_android.cc
+++ b/runtime/bin/process_android.cc
@@ -436,24 +436,24 @@
}
}
- // Tries to find path_ relative to the current namespace.
+ // Tries to find path_ relative to the current namespace unless it should be
+ // searched in the PATH.
// The path that should be passed to exec is returned in realpath.
// Returns true on success, and false if there was an error that should
// be reported to the parent.
bool FindPathInNamespace(char* realpath, intptr_t realpath_size) {
+ // Perform a PATH search if there's no slash in the path.
+ if (strchr(path_, '/') == NULL) {
+ // TODO(zra): If there is a non-default namespace, the entries in PATH
+ // should be treated as relative to the namespace.
+ strncpy(realpath, path_, realpath_size);
+ realpath[realpath_size - 1] = '\0';
+ return true;
+ }
NamespaceScope ns(namespc_, path_);
const int fd =
TEMP_FAILURE_RETRY(openat(ns.fd(), ns.path(), O_RDONLY | O_CLOEXEC));
if (fd == -1) {
- if ((errno == ENOENT) && (strchr(path_, '/') == NULL)) {
- // path_ was not found relative to the namespace, but since it didn't
- // contain a '/', we can pass it directly to execvp, which will do a
- // lookup in PATH.
- // TODO(zra): If there is a non-default namespace, the entries in PATH
- // should be treated as relative to the namespace.
- strncpy(realpath, path_, realpath_size);
- return true;
- }
return false;
}
char procpath[PATH_MAX];
diff --git a/runtime/bin/process_linux.cc b/runtime/bin/process_linux.cc
index 93c7f6c..d3df465 100644
--- a/runtime/bin/process_linux.cc
+++ b/runtime/bin/process_linux.cc
@@ -436,24 +436,24 @@
}
}
- // Tries to find path_ relative to the current namespace.
+ // Tries to find path_ relative to the current namespace unless it should be
+ // searched in the PATH.
// The path that should be passed to exec is returned in realpath.
// Returns true on success, and false if there was an error that should
// be reported to the parent.
bool FindPathInNamespace(char* realpath, intptr_t realpath_size) {
+ // Perform a PATH search if there's no slash in the path.
+ if (strchr(path_, '/') == NULL) {
+ // TODO(zra): If there is a non-default namespace, the entries in PATH
+ // should be treated as relative to the namespace.
+ strncpy(realpath, path_, realpath_size);
+ realpath[realpath_size - 1] = '\0';
+ return true;
+ }
NamespaceScope ns(namespc_, path_);
const int fd =
TEMP_FAILURE_RETRY(openat64(ns.fd(), ns.path(), O_RDONLY | O_CLOEXEC));
if (fd == -1) {
- if ((errno == ENOENT) && (strchr(path_, '/') == NULL)) {
- // path_ was not found relative to the namespace, but since it didn't
- // contain a '/', we can pass it directly to execvp, which will do a
- // lookup in PATH.
- // TODO(zra): If there is a non-default namespace, the entries in PATH
- // should be treated as relative to the namespace.
- strncpy(realpath, path_, realpath_size);
- return true;
- }
return false;
}
char procpath[PATH_MAX];
