abstractdog commented on code in PR #537:
URL: https://github.com/apache/tez/pull/537#discussion_r4061150838
##########
tez-common/src/main/java/org/apache/tez/dag/utils/RelocalizationUtils.java:
##########
@@ -60,11 +60,37 @@ public static void addUrlsToClassPath(List<URL> urls) {
private static Path downloadResource(String destName, URI uri, Configuration
conf, String destDir)
throws IOException {
+ // destName is the map key from the client-supplied additionalAmResources
+ // and ends up as the file name we write under the AM working directory.
+ // Refuse anything that could redirect that write outside destDir — an
+ // absolute path, a path separator, or a ".." segment — so a submitter
+ // holding AM modify access cannot land a jar anywhere on the AM host and
+ // chain it into code execution via the classloader / am-hooks path.
+ validateDestName(destName);
FileSystem fs = FileSystem.get(uri, conf);
Path cwd = new Path(destDir);
Path dFile = new Path(cwd, destName);
Path srcPath = new Path(uri);
fs.copyToLocalFile(srcPath, dFile);
return dFile.makeQualified(FileSystem.getLocal(conf).getUri(), cwd);
}
+
+ static void validateDestName(String destName) {
+ if (destName == null || destName.isEmpty()) {
+ throw new IllegalArgumentException("Resource name must not be empty");
+ }
+ if (destName.indexOf('/') >= 0 || destName.indexOf('\\') >= 0
+ || destName.indexOf('\0') >= 0) {
+ throw new IllegalArgumentException(
+ "Resource name must not contain path separators: " + destName);
+ }
+ if (destName.equals(".") || destName.equals("..")) {
+ throw new IllegalArgumentException(
+ "Resource name must not be a parent-directory reference: " +
destName);
+ }
+ if (new Path(destName).isAbsolute()) {
Review Comment:
not true, adding a unit test to prove: `uriSchemeOpaqueFormIsRejected`
`new Path(destName)` call inside `RelocalizationUtils#validateDestName`
already throws`IllegalArgumentException` from Hadoop's URI parser
##########
tez-common/src/main/java/org/apache/tez/dag/utils/RelocalizationUtils.java:
##########
@@ -60,11 +60,37 @@ public static void addUrlsToClassPath(List<URL> urls) {
private static Path downloadResource(String destName, URI uri, Configuration
conf, String destDir)
throws IOException {
+ // destName is the map key from the client-supplied additionalAmResources
+ // and ends up as the file name we write under the AM working directory.
+ // Refuse anything that could redirect that write outside destDir — an
+ // absolute path, a path separator, or a ".." segment — so a submitter
+ // holding AM modify access cannot land a jar anywhere on the AM host and
+ // chain it into code execution via the classloader / am-hooks path.
+ validateDestName(destName);
FileSystem fs = FileSystem.get(uri, conf);
Path cwd = new Path(destDir);
Path dFile = new Path(cwd, destName);
Path srcPath = new Path(uri);
fs.copyToLocalFile(srcPath, dFile);
return dFile.makeQualified(FileSystem.getLocal(conf).getUri(), cwd);
}
+
+ static void validateDestName(String destName) {
+ if (destName == null || destName.isEmpty()) {
+ throw new IllegalArgumentException("Resource name must not be empty");
+ }
+ if (destName.indexOf('/') >= 0 || destName.indexOf('\\') >= 0
+ || destName.indexOf('\0') >= 0) {
+ throw new IllegalArgumentException(
+ "Resource name must not contain path separators: " + destName);
+ }
+ if (destName.equals(".") || destName.equals("..")) {
+ throw new IllegalArgumentException(
+ "Resource name must not be a parent-directory reference: " +
destName);
+ }
+ if (new Path(destName).isAbsolute()) {
Review Comment:
not true, adding a unit test to prove: `uriSchemeOpaqueFormIsRejected`
`new Path(destName)` call inside `RelocalizationUtils#validateDestName`
already throws`IllegalArgumentException` from Hadoop's URI parser in such cases
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
