diff --git a/tez-common/src/main/java/org/apache/tez/dag/utils/RelocalizationUtils.java b/tez-common/src/main/java/org/apache/tez/dag/utils/RelocalizationUtils.java index 439f0d275e..0e7938e5ed 100644 --- a/tez-common/src/main/java/org/apache/tez/dag/utils/RelocalizationUtils.java +++ b/tez-common/src/main/java/org/apache/tez/dag/utils/RelocalizationUtils.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.net.URI; import java.net.URL; +import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Map; @@ -58,8 +59,19 @@ public static void addUrlsToClassPath(List urls) { ReflectionUtils.addResourcesToSystemClassLoader(urls); } + /** Validate all names up front, so callers can reject before mutating state. */ + public static void validateDestNames(Collection destNames) { + if (destNames == null) { + return; + } + for (String destName : destNames) { + validateDestName(destName); + } + } + private static Path downloadResource(String destName, URI uri, Configuration conf, String destDir) throws IOException { + validateDestName(destName); FileSystem fs = FileSystem.get(uri, conf); Path cwd = new Path(destDir); Path dFile = new Path(cwd, destName); @@ -67,4 +79,23 @@ private static Path downloadResource(String destName, URI uri, Configuration con 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()) { + throw new IllegalArgumentException( + "Resource name must not be absolute: " + destName); + } + } } diff --git a/tez-common/src/test/java/org/apache/tez/dag/utils/TestRelocalizationUtils.java b/tez-common/src/test/java/org/apache/tez/dag/utils/TestRelocalizationUtils.java new file mode 100644 index 0000000000..272e344b6e --- /dev/null +++ b/tez-common/src/test/java/org/apache/tez/dag/utils/TestRelocalizationUtils.java @@ -0,0 +1,99 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tez.dag.utils; + +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.Arrays; +import java.util.Collections; + +import org.apache.hadoop.fs.Path; + +import org.junit.jupiter.api.Test; + +public class TestRelocalizationUtils { + + @Test + public void plainFileNameIsAccepted() { + assertDoesNotThrow(() -> RelocalizationUtils.validateDestName("lib.jar")); + assertDoesNotThrow(() -> RelocalizationUtils.validateDestName("some-name_1.tar.gz")); + } + + @Test + public void traversalIsRejected() { + // Any path separator or parent-directory reference must be refused: those + // are the shapes that let a submitter's key redirect the AM download to + // a location outside its working directory. + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName("../evil.jar")); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName("dir/child.jar")); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName("dir\\child.jar")); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName("/absolute/path.jar")); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName(".")); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName("..")); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName("")); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName(null)); + } + + @Test + public void validateDestNamesRejectsAnyBadNameInBatch() { + // One bad name must fail the whole batch, before any download starts. + assertDoesNotThrow(() -> RelocalizationUtils.validateDestNames( + Arrays.asList("a.jar", "b.jar"))); + assertDoesNotThrow(() -> RelocalizationUtils.validateDestNames(null)); + assertDoesNotThrow( + () -> RelocalizationUtils.validateDestNames(Collections.emptyList())); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestNames( + Arrays.asList("ok.jar", "../evil.jar"))); + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestNames( + Arrays.asList("a.jar", "/absolute.jar", "b.jar"))); + } + + /** + * A URI-scheme opaque form such as {@code "file:.."} is rejected today: the + * {@code new Path(destName)} call inside {@link RelocalizationUtils#validateDestName} + * throws {@link IllegalArgumentException} from Hadoop's URI parser + * ("Relative path in absolute URI"). And {@code new Path(cwd, "file:..")} + * — the very next construction in {@code downloadResource} — throws + * identically, so the value cannot reach {@code copyToLocalFile} and + * cannot resolve outside {@code cwd}. + */ + @Test + public void uriSchemeOpaqueFormIsRejected() { + for (String s : new String[]{"file:..", "file:../evil.jar", "file:evil.jar", + "mailto:x", "C:evil.jar"}) { + assertThrows(IllegalArgumentException.class, + () -> RelocalizationUtils.validateDestName(s), + "validateDestName should reject " + s); + assertThrows(IllegalArgumentException.class, + () -> new Path(new Path("/tmp/work"), s), + "new Path(cwd, s) should also throw for " + s); + } + } +} diff --git a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java index 1ef4ccbc92..d613fe4ccf 100644 --- a/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java +++ b/tez-dag/src/main/java/org/apache/tez/dag/app/DAGAppMaster.java @@ -1348,6 +1348,15 @@ public Void run() throws Exception { public String submitDAGToAppMaster(DAGPlan dagPlan, Map additionalResources) throws TezException { + // Validate before startDAGExecution mutates currentDAG and amResources: + // a later failure would leave the session stuck "already running a DAG". + if (additionalResources != null) { + try { + RelocalizationUtils.validateDestNames(additionalResources.keySet()); + } catch (IllegalArgumentException e) { + throw new TezException(e); + } + } appMasterReadinessService.waitToBeReady(); if (sessionStopped.get()) {