diff --git a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/executable/ExecutableManager.java b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/executable/ExecutableManager.java index a9f8ffc42296f..b7ab16b384fb3 100644 --- a/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/executable/ExecutableManager.java +++ b/iotdb-core/node-commons/src/main/java/org/apache/iotdb/commons/executable/ExecutableManager.java @@ -99,22 +99,62 @@ private void downloadExecutables(List uris, long requestId) // endregion + // ====================================================== + // region path containment + // ====================================================== + + /** + * Resolves {@code fileName} against {@code root} and verifies the result stays inside it. + * + *

These directories are addressed by a caller-supplied name, and a name carrying {@code ..} + * segments or an absolute path would otherwise resolve outside the managed directory. Resolving + * and normalizing first, then checking containment, keeps every accessor confined to its root + * regardless of what the name contains. + * + * @throws IOException if the resolved path escapes {@code root} + */ + public static Path resolveUnderRoot(String root, String fileName) throws IOException { + Path rootPath = Paths.get(root).toAbsolutePath().normalize(); + Path resolved = rootPath.resolve(fileName).normalize(); + if (!resolved.startsWith(rootPath)) { + throw new IOException( + String.format("The resolved path %s is outside of the directory %s", resolved, rootPath)); + } + return resolved; + } + + private Path resolveUnderLibRoot(String fileName) throws IOException { + return resolveUnderRoot(this.libRoot, fileName); + } + + private Path resolveUnderTemporaryRoot(String fileName) throws IOException { + return resolveUnderRoot(this.temporaryLibRoot, fileName); + } + + // endregion + // ====================================================== // region File under LibRoot // ====================================================== public void removeFileUnderLibRoot(String fileName) throws IOException { - Path path = Paths.get(this.libRoot + File.separator + fileName); - Files.deleteIfExists(path); + Files.deleteIfExists(resolveUnderLibRoot(fileName)); } public boolean hasFileUnderLibRoot(String fileName) { - return Files.exists(Paths.get(this.libRoot + File.separator + fileName)); + try { + return Files.exists(resolveUnderLibRoot(fileName)); + } catch (IOException e) { + return false; + } } public boolean hasFileUnderInstallDir(String fileName) { - return Files.exists( - Paths.get(this.libRoot + File.separator + INSTALL_DIR + File.separator + fileName)); + try { + return Files.exists(resolveUnderLibRoot(INSTALL_DIR + File.separator + fileName)); + } catch (IOException e) { + return false; + } } // endregion @@ -124,7 +164,11 @@ public boolean hasFileUnderInstallDir(String fileName) { // ====================================================== public boolean hasFileUnderTemporaryRoot(String fileName) { - return Files.exists(Paths.get(this.temporaryLibRoot + File.separator + fileName)); + try { + return Files.exists(resolveUnderTemporaryRoot(fileName)); + } catch (IOException e) { + return false; + } } private void removeFromTemporaryLibRoot(long requestId) { @@ -132,19 +176,17 @@ private void removeFromTemporaryLibRoot(long requestId) { } public void saveTextAsFileUnderTemporaryRoot(String text, String fileName) throws IOException { - Path path = Paths.get(this.temporaryLibRoot + File.separator + fileName); + Path path = resolveUnderTemporaryRoot(fileName); Files.deleteIfExists(path); Files.write(path, text.getBytes()); } public void removeFileUnderTemporaryRoot(String fileName) throws IOException { - Path path = Paths.get(this.temporaryLibRoot + File.separator + fileName); - Files.deleteIfExists(path); + Files.deleteIfExists(resolveUnderTemporaryRoot(fileName)); } public String readTextFromFileUnderTemporaryRoot(String fileName) throws IOException { - Path path = Paths.get(this.temporaryLibRoot + File.separator + fileName); - return new String(Files.readAllBytes(path)); + return new String(Files.readAllBytes(resolveUnderTemporaryRoot(fileName))); } // endregion diff --git a/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/executable/ExecutableManagerTest.java b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/executable/ExecutableManagerTest.java new file mode 100644 index 0000000000000..8f56c947364c8 --- /dev/null +++ b/iotdb-core/node-commons/src/test/java/org/apache/iotdb/commons/executable/ExecutableManagerTest.java @@ -0,0 +1,171 @@ +/* + * 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.iotdb.commons.executable; + +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.Comparator; +import java.util.stream.Stream; + +public class ExecutableManagerTest { + + private static final String TEST_ROOT = + "target".concat(File.separator).concat("ExecutableManagerTest"); + private static final String TEMPORARY_ROOT = TEST_ROOT.concat(File.separator).concat("tmp"); + private static final String LIB_ROOT = TEST_ROOT.concat(File.separator).concat("lib"); + + private ExecutableManager executableManager; + + @Before + public void setUp() throws Exception { + executableManager = new ExecutableManager(TEMPORARY_ROOT, LIB_ROOT); + Files.createDirectories(Paths.get(TEMPORARY_ROOT)); + Files.createDirectories(Paths.get(LIB_ROOT)); + } + + @After + public void tearDown() throws Exception { + final Path root = Paths.get(TEST_ROOT); + if (!Files.exists(root)) { + return; + } + try (final Stream paths = Files.walk(root)) { + paths.sorted(Comparator.reverseOrder()).forEach(path -> path.toFile().delete()); + } + } + + @Test + public void testResolveUnderRootAcceptsNamesInsideTheRoot() throws IOException { + final Path root = Paths.get(LIB_ROOT).toAbsolutePath().normalize(); + + Assert.assertEquals( + root.resolve("udf.jar"), ExecutableManager.resolveUnderRoot(LIB_ROOT, "udf.jar")); + Assert.assertEquals( + root.resolve("install").resolve("udf.jar"), + ExecutableManager.resolveUnderRoot(LIB_ROOT, "install".concat(File.separator).concat("udf.jar"))); + // a name that walks out and back in still resolves inside the root + Assert.assertEquals( + root.resolve("udf.jar"), + ExecutableManager.resolveUnderRoot(LIB_ROOT, "sub".concat(File.separator).concat("..").concat(File.separator).concat("udf.jar"))); + } + + @Test + public void testResolveUnderRootRejectsNamesOutsideTheRoot() { + final String[] escapingNames = + new String[] { + "..".concat(File.separator).concat("escaped.jar"), + "..".concat(File.separator).concat("..").concat(File.separator).concat("escaped.jar"), + "sub" + .concat(File.separator) + .concat("..") + .concat(File.separator) + .concat("..") + .concat(File.separator) + .concat("escaped.jar"), + File.separator.concat("tmp").concat(File.separator).concat("escaped.jar"), + }; + + for (final String name : escapingNames) { + try { + ExecutableManager.resolveUnderRoot(LIB_ROOT, name); + Assert.fail("expected the name to be rejected: ".concat(name)); + } catch (final IOException expected) { + // the resolved path is outside the root + } + } + } + + @Test + public void testSaveTextUnderTemporaryRootStaysInsideTheRoot() throws IOException { + executableManager.saveTextAsFileUnderTemporaryRoot("content", "plugin.txt"); + Assert.assertTrue(executableManager.hasFileUnderTemporaryRoot("plugin.txt")); + Assert.assertEquals( + "content", executableManager.readTextFromFileUnderTemporaryRoot("plugin.txt")); + } + + @Test + public void testSaveTextUnderTemporaryRootRejectsAnEscapingName() { + final String escaping = + "..".concat(File.separator).concat("..").concat(File.separator).concat("escaped.txt"); + try { + executableManager.saveTextAsFileUnderTemporaryRoot("content", escaping); + Assert.fail("expected the name to be rejected"); + } catch (final IOException expected) { + // the resolved path is outside the temporary root + } + Assert.assertFalse( + Files.exists(Paths.get(TEST_ROOT).toAbsolutePath().normalize().resolve("escaped.txt"))); + } + + @Test + public void testRemoveAndReadUnderTemporaryRootRejectAnEscapingName() { + final String escaping = "..".concat(File.separator).concat("escaped.txt"); + try { + executableManager.removeFileUnderTemporaryRoot(escaping); + Assert.fail("expected the name to be rejected"); + } catch (final IOException expected) { + // expected + } + try { + executableManager.readTextFromFileUnderTemporaryRoot(escaping); + Assert.fail("expected the name to be rejected"); + } catch (final IOException expected) { + // expected + } + } + + @Test + public void testRemoveUnderLibRootRejectsAnEscapingName() throws IOException { + final Path outside = + Paths.get(TEST_ROOT).toAbsolutePath().normalize().resolve("outside-lib.jar"); + Files.write(outside, "keep".getBytes()); + + try { + executableManager.removeFileUnderLibRoot( + "..".concat(File.separator).concat("outside-lib.jar")); + Assert.fail("expected the name to be rejected"); + } catch (final IOException expected) { + // expected + } + Assert.assertTrue("the file outside the lib root must not be deleted", Files.exists(outside)); + } + + @Test + public void testHasFileAccessorsReturnFalseForAnEscapingName() throws IOException { + final Path outside = + Paths.get(TEST_ROOT).toAbsolutePath().normalize().resolve("outside-probe.jar"); + Files.write(outside, "probe".getBytes()); + + final String escaping = "..".concat(File.separator).concat("outside-probe.jar"); + Assert.assertFalse(executableManager.hasFileUnderLibRoot(escaping)); + Assert.assertFalse(executableManager.hasFileUnderTemporaryRoot(escaping)); + Assert.assertFalse( + executableManager.hasFileUnderInstallDir( + "..".concat(File.separator).concat("..").concat(File.separator).concat("outside-probe.jar"))); + } +}