Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
import com.google.devtools.build.lib.util.io.FileOutErr;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.io.InputStream;
import java.time.Duration;
Expand Down Expand Up @@ -363,6 +364,10 @@ protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, S

String testTmpdir = env.get("TEST_TMPDIR");
if (testTmpdir != null) {
// Validate that TEST_TMPDIR does not contain path traversal sequences.
// Unlike TMPDIR (sanitized by PosixLocalEnvProvider, see #3296),
// TEST_TMPDIR is passed through unvalidated from the action environment.
validateTestTmpdir(testTmpdir, sandboxExecRoot);
addWritablePath(
sandboxExecRoot,
writablePaths,
Expand Down Expand Up @@ -434,6 +439,24 @@ private static void addWritablePath(
}
}

/**
* Validates that TEST_TMPDIR does not contain path traversal sequences.
*
* <p>Absolute TEST_TMPDIR values are legitimate (set via --test_tmpdir).
* The concern is '../' traversal in relative paths escaping sandboxExecRoot.
*/
private static void validateTestTmpdir(String testTmpdir, Path sandboxExecRoot)
throws IOException {
PathFragment fragment = PathFragment.create(testTmpdir);
if (fragment.containsUplevelReferences()) {
throw new IOException(
String.format(
"TEST_TMPDIR must not contain '..' path traversal (it could escape the sandbox),"
+ " got: \"%s\" (sandboxExecRoot: %s)",
testTmpdir, sandboxExecRoot.getPathString()));
}
}

protected ImmutableSet<Path> getInaccessiblePaths() {
return inaccessiblePaths;
}
Expand Down
Loading