diff --git a/hetu-common/src/main/java/io/hetu/core/common/util/SecureObjectInputStream.java b/hetu-common/src/main/java/io/hetu/core/common/util/SecureObjectInputStream.java index ef92dc450..70d97cb1d 100644 --- a/hetu-common/src/main/java/io/hetu/core/common/util/SecureObjectInputStream.java +++ b/hetu-common/src/main/java/io/hetu/core/common/util/SecureObjectInputStream.java @@ -1,4 +1,5 @@ /* + * Copyright (C) 2018-2020. Huawei Technologies Co., Ltd. All rights reserved. * Licensed 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 diff --git a/hetu-common/src/main/java/io/hetu/core/common/util/SecurePathWhiteList.java b/hetu-common/src/main/java/io/hetu/core/common/util/SecurePathWhiteList.java new file mode 100644 index 000000000..b9dc5c1f2 --- /dev/null +++ b/hetu-common/src/main/java/io/hetu/core/common/util/SecurePathWhiteList.java @@ -0,0 +1,50 @@ +/* + * Copyright (C) 2018-2020. Huawei Technologies Co., Ltd. All rights reserved. + * Licensed 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 io.hetu.core.common.util; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +public class SecurePathWhiteList +{ + private SecurePathWhiteList() + { + } + + public static List getSecurePathWhiteList() throws IOException + { + return new ArrayList<>(Arrays.asList( + new File("..").getCanonicalPath(), + "/tmp")); + } + + public static boolean isSecurePath(String absolutePath) throws IOException + { + // absolutePath + if (absolutePath.startsWith("/")) { + return getSecurePathWhiteList().stream() + .filter(securePath -> absolutePath.startsWith(securePath)) + .findAny() + .isPresent(); + } + // currentDirectory + else { + return true; + } + } +} diff --git a/hetu-hbase/src/main/java/io/hetu/core/plugin/hbase/query/HBaseGetRecordCursor.java b/hetu-hbase/src/main/java/io/hetu/core/plugin/hbase/query/HBaseGetRecordCursor.java index a148b6091..3a40d86ad 100644 --- a/hetu-hbase/src/main/java/io/hetu/core/plugin/hbase/query/HBaseGetRecordCursor.java +++ b/hetu-hbase/src/main/java/io/hetu/core/plugin/hbase/query/HBaseGetRecordCursor.java @@ -18,7 +18,9 @@ import io.airlift.log.Logger; import io.airlift.slice.Slice; import io.hetu.core.plugin.hbase.connector.HBaseColumnHandle; import io.hetu.core.plugin.hbase.split.HBaseSplit; +import io.hetu.core.plugin.hbase.utils.HBaseErrorCode; import io.hetu.core.plugin.hbase.utils.serializers.HBaseRowSerializer; +import io.prestosql.spi.PrestoException; import io.prestosql.spi.connector.ColumnHandle; import io.prestosql.spi.predicate.Range; import io.prestosql.spi.type.Type; @@ -129,17 +131,23 @@ public class HBaseGetRecordCursor @Override public boolean advanceNextPosition() { - if (this.currentRecordIndex >= this.results.length) { - return false; - } - else { - Result record = this.results[this.currentRecordIndex]; - serializer.reset(); - if (record.getRow() != null) { - serializer.deserialize(record, this.defaultValue); + try { + if (this.currentRecordIndex >= this.results.length) { + return false; } - this.currentRecordIndex++; - return true; + else { + Result record = this.results[this.currentRecordIndex]; + serializer.reset(); + if (record.getRow() != null) { + serializer.deserialize(record, this.defaultValue); + } + this.currentRecordIndex++; + return true; + } + } + catch (Exception e) { + this.close(); + throw new PrestoException(HBaseErrorCode.IO_ERROR, e); } } diff --git a/hetu-heuristic-index-cli/pom.xml b/hetu-heuristic-index-cli/pom.xml index 1f8ee06f7..3074676eb 100644 --- a/hetu-heuristic-index-cli/pom.xml +++ b/hetu-heuristic-index-cli/pom.xml @@ -69,6 +69,10 @@ + + io.hetu.core + hetu-common + com.google.guava guava diff --git a/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommand.java b/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommand.java index f73446fce..e6549a953 100644 --- a/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommand.java +++ b/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommand.java @@ -15,6 +15,7 @@ package io.hetu.core.heuristicindex; +import io.hetu.core.common.util.SecurePathWhiteList; import io.prestosql.spi.heuristicindex.IndexClient; import io.prestosql.spi.heuristicindex.IndexFactory; import io.prestosql.spi.heuristicindex.IndexMetadata; @@ -78,8 +79,7 @@ public class IndexCommand @CommandLine.Option( names = {"-c", "--config"}, required = true, - defaultValue = "../etc", - description = "root folder of hetu etc directory (default: ${DEFAULT-VALUE})") + description = "root folder of hetu etc directory") String configDirPath; @CommandLine.Option( names = {"-t", "--table"}, @@ -131,6 +131,13 @@ public class IndexCommand { } + public IndexCommand(String configDirPath, String table, Command command) + { + this.configDirPath = configDirPath; + this.table = table; + this.command = command; + } + /** * start application * @@ -147,9 +154,24 @@ public class IndexCommand public Void call() throws IOException { - // make sure the file paths provided exist + // validate inputs + // security check required before using values in Path + // e.g. catalog.schema.table or dc.catalog.schema.table + checkArgument(table.matches("([\\p{Alnum}_]+\\.){2,3}[\\p{Alnum}_]+"), "Invalid table name"); + + if (columns != null) { + for (String column : columns) { + checkArgument(column.matches("[\\p{Alnum}_]+"), "Invalid column name"); + } + } + + checkArgument(!configDirPath.contains("../"), + "Config directory path must be absolute or current directory and at user workspace: " + SecurePathWhiteList.getSecurePathWhiteList().toString()); checkArgument(Paths.get(configDirPath).toFile().exists(), "Config directory does not exist"); + checkArgument(SecurePathWhiteList.isSecurePath(configDirPath), + "Config directory path must at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + IndexFactory factory = IndexCommandUtils.getIndexFactory(); // based on the command, different values are required diff --git a/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommandUtils.java b/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommandUtils.java index da3a094fa..254d1b0fe 100644 --- a/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommandUtils.java +++ b/hetu-heuristic-index-cli/src/main/java/io/hetu/core/heuristicindex/IndexCommandUtils.java @@ -15,6 +15,7 @@ package io.hetu.core.heuristicindex; import com.google.common.collect.ImmutableSet; +import io.hetu.core.common.util.SecurePathWhiteList; import io.hetu.core.filesystem.HdfsFileSystemClientFactory; import io.hetu.core.filesystem.LocalFileSystemClientFactory; import io.hetu.core.heuristicindex.util.IndexConstants; @@ -33,6 +34,7 @@ import java.nio.file.Paths; import java.util.Properties; import java.util.Set; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; public class IndexCommandUtils @@ -86,6 +88,15 @@ public class IndexCommandUtils Path root = Paths.get(requireNonNull(properties.getProperty(IndexConstants.INDEXSTORE_URI_KEY), IndexConstants.INDEXSTORE_URI_KEY + " is not set in config.properties")); + try { + checkArgument(!root.toString().contains("../"), "Index store directory path must be absolute"); + checkArgument(SecurePathWhiteList.isSecurePath(root.toString()), + "Index store directory path must be at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new IllegalArgumentException("Failed to get secure path list.", e); + } + String fileSystemProfileName = requireNonNull(properties.getProperty(IndexConstants.INDEXSTORE_FILESYSTEM_PROFILE_KEY), IndexConstants.INDEXSTORE_FILESYSTEM_PROFILE_KEY + " is not set in config.properties"); diff --git a/hetu-heuristic-index-cli/src/test/java/io/hetu/core/heuristicindex/TestIndexCommand.java b/hetu-heuristic-index-cli/src/test/java/io/hetu/core/heuristicindex/TestIndexCommand.java index 743331d6c..b8199c40b 100644 --- a/hetu-heuristic-index-cli/src/test/java/io/hetu/core/heuristicindex/TestIndexCommand.java +++ b/hetu-heuristic-index-cli/src/test/java/io/hetu/core/heuristicindex/TestIndexCommand.java @@ -54,6 +54,18 @@ import static org.testng.Assert.assertTrue; public class TestIndexCommand extends PowerMockTestCase { + @Test + public void validateInputs() throws IOException + { + try { + IndexCommand indexCommand = new IndexCommand("/", "catalog.schema.table", IndexCommand.Command.show); + indexCommand.call(); + } + catch (IllegalArgumentException e) { + assertTrue(e.getMessage().contains("Config directory path must at user workspace")); + } + } + @Test public void testCallWithEmptyConfigDirectory() throws IOException diff --git a/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexWriter.java b/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexWriter.java index 8d4cb60c3..e9c7a4bd6 100644 --- a/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexWriter.java +++ b/hetu-heuristic-index/src/main/java/io/hetu/core/heuristicindex/HeuristicIndexWriter.java @@ -14,6 +14,7 @@ */ package io.hetu.core.heuristicindex; +import io.hetu.core.common.util.SecurePathWhiteList; import io.hetu.core.filesystem.HetuLocalFileSystemClient; import io.hetu.core.filesystem.LocalConfig; import io.hetu.core.heuristicindex.util.IndexConstants; @@ -47,6 +48,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.locks.Lock; import java.util.stream.Stream; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; /** @@ -127,6 +129,8 @@ public class HeuristicIndexWriter // lock table so multiple callers can't index the same table Path tableIndexDirPath = Paths.get(strTmpPath, root.toString(), table); + checkArgument(SecurePathWhiteList.isSecurePath(tableIndexDirPath.toString()), + "Create index temp directory path must be at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); Lock lock = null; if (lockingEnabled) { @@ -161,6 +165,19 @@ public class HeuristicIndexWriter return; } + // security check required before using values in a Path + if (!column.matches("[\\p{Alnum}_]+")) { + LOG.warn("Invalid column name " + column); + return; + } + try { + checkArgument(SecurePathWhiteList.isSecurePath(tableIndexDirPath.toString()), + "Create index temp directory path must be at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new UncheckedIOException("Get secure path list error", e); + } + Path columnIndexDirPath = tableIndexDirPath.resolve(column); indexedColumns.add(column); diff --git a/hetu-metastore/pom.xml b/hetu-metastore/pom.xml index aec87a295..4c9ac20dd 100644 --- a/hetu-metastore/pom.xml +++ b/hetu-metastore/pom.xml @@ -99,6 +99,11 @@ runtime + + io.hetu.core + hetu-common + + io.hetu.core presto-spi diff --git a/hetu-metastore/src/main/java/io/hetu/core/metastore/hetufilesystem/HetuFsMetastore.java b/hetu-metastore/src/main/java/io/hetu/core/metastore/hetufilesystem/HetuFsMetastore.java index dd44b2b6a..5f601df4c 100644 --- a/hetu-metastore/src/main/java/io/hetu/core/metastore/hetufilesystem/HetuFsMetastore.java +++ b/hetu-metastore/src/main/java/io/hetu/core/metastore/hetufilesystem/HetuFsMetastore.java @@ -17,6 +17,7 @@ package io.hetu.core.metastore.hetufilesystem; import com.google.common.io.CharStreams; import io.airlift.json.JsonCodec; import io.airlift.log.Logger; +import io.hetu.core.common.util.SecurePathWhiteList; import io.hetu.core.metastore.jdbc.JdbcMetadataUtil; import io.prestosql.spi.PrestoException; import io.prestosql.spi.connector.CatalogAlreadyExistsException; @@ -50,6 +51,7 @@ import java.util.concurrent.locks.Lock; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.google.common.base.Preconditions.checkArgument; import static io.prestosql.spi.metastore.HetuErrorCode.HETU_METASTORE_CODE; import static java.nio.charset.StandardCharsets.UTF_8; @@ -86,6 +88,17 @@ public class HetuFsMetastore { this.metadataPath = metadataConfig.getHetuFileSystemMetastorePath(); this.client = client; + + try { + checkArgument(!metadataPath.contains("../"), + "Metadata directory path must be absolute and at user workspace: " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + checkArgument(SecurePathWhiteList.isSecurePath(metadataPath), + "Metadata directory path must be at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new IllegalArgumentException("Failed to get secure path list.", e); + } + if (!client.exists(Paths.get(metadataPath))) { try { client.createDirectories(Paths.get(metadataPath)); @@ -141,6 +154,8 @@ public class HetuFsMetastore @Override public void createCatalog(CatalogEntity catalog) { + checkArgument(catalog.getName().matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + runTransaction(() -> { assertCatalogNotExist(catalog.getName()); try (OutputStream outputStream = client.newOutputStream(getCatalogMetadataPath(catalog.getName()))) { @@ -155,6 +170,9 @@ public class HetuFsMetastore @Override public void alterCatalog(String catalogName, CatalogEntity newCatalog) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(newCatalog.getName().matches("[\\p{Alnum}_]+"), "Invalid new catalog name"); + runTransaction(() -> { if (!catalogName.equals(newCatalog.getName())) { throw new PrestoException(HETU_METASTORE_CODE, "Cannot alter a catalog's name"); @@ -181,6 +199,8 @@ public class HetuFsMetastore @Override public void dropCatalog(String catalogName) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + runTransaction(() -> { assertCatalogExist(catalogName); Path catalogMetadataDir = getCatalogMetadataDir(catalogName); @@ -218,6 +238,8 @@ public class HetuFsMetastore @Override public Optional getCatalog(String catalogName) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + try { assertCatalogExist(catalogName); } @@ -285,6 +307,9 @@ public class HetuFsMetastore @Override public void createDatabase(DatabaseEntity database) { + checkArgument(database.getName().matches("[\\p{Alnum}_]+"), "Invalid database name"); + checkArgument(database.getCatalogName().matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + runTransaction(() -> { try { assertCatalogExist(database.getCatalogName()); @@ -307,6 +332,10 @@ public class HetuFsMetastore @Override public void alterDatabase(String catalogName, String databaseName, DatabaseEntity newDatabase) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + checkArgument(newDatabase.getName().matches("[\\p{Alnum}_]+"), "Invalid new database name"); + runTransaction(() -> { if (!catalogName.equals(newDatabase.getCatalogName())) { throw new PrestoException(HETU_METASTORE_CODE, "The catalog name is not correct"); @@ -363,6 +392,9 @@ public class HetuFsMetastore @Override public void dropDatabase(String catalogName, String databaseName) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + runTransaction(() -> { assertCatalogExist(catalogName); assertDatabaseExist(catalogName, databaseName); @@ -401,6 +433,9 @@ public class HetuFsMetastore @Override public Optional getDatabase(String catalogName, String databaseName) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + try { assertCatalogExist(catalogName); assertDatabaseExist(catalogName, databaseName); @@ -421,6 +456,8 @@ public class HetuFsMetastore @Override public List getAllDatabases(String catalogName) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + List databases = new ArrayList<>(); assertCatalogExist(catalogName); try (Stream paths = client.list(getCatalogMetadataDir(catalogName))) { @@ -471,6 +508,10 @@ public class HetuFsMetastore String databaseName = table.getDatabaseName(); String tableName = table.getName(); + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + checkArgument(tableName.matches("[\\p{Alnum}_]+"), "Invalid table name"); + assertCatalogExist(catalogName); assertDatabaseExist(catalogName, databaseName); assertTableNotExist(catalogName, databaseName, tableName); @@ -487,6 +528,10 @@ public class HetuFsMetastore @Override public void dropTable(String catalogName, String databaseName, String tableName) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + checkArgument(tableName.matches("[\\p{Alnum}_]+"), "Invalid table name"); + runTransaction(() -> { assertCatalogExist(catalogName); assertDatabaseExist(catalogName, databaseName); @@ -504,6 +549,11 @@ public class HetuFsMetastore @Override public void alterTable(String catalogName, String databaseName, String oldTableName, TableEntity newTable) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + checkArgument(oldTableName.matches("[\\p{Alnum}_]+"), "Invalid table name"); + checkArgument(newTable.getName().matches("[\\p{Alnum}_]+"), "Invalid new table name"); + runTransaction(() -> { if (!catalogName.equals(newTable.getCatalogName()) || !databaseName.equals(newTable.getDatabaseName())) { throw new PrestoException(HETU_METASTORE_CODE, "The catalog name or schema name is not correct"); @@ -539,6 +589,10 @@ public class HetuFsMetastore @Override public Optional getTable(String catalogName, String databaseName, String table) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + checkArgument(table.matches("[\\p{Alnum}_]+"), "Invalid table name"); + try { assertCatalogExist(catalogName); assertDatabaseExist(catalogName, databaseName); @@ -560,6 +614,9 @@ public class HetuFsMetastore @Override public List getAllTables(String catalogName, String databaseName) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + checkArgument(databaseName.matches("[\\p{Alnum}_]+"), "Invalid database name"); + List tables = new ArrayList<>(); assertCatalogExist(catalogName); diff --git a/hetu-seed-store/src/main/java/io/hetu/core/seedstore/filebased/FileBasedSeedStore.java b/hetu-seed-store/src/main/java/io/hetu/core/seedstore/filebased/FileBasedSeedStore.java index b6068f766..2494ad5c6 100644 --- a/hetu-seed-store/src/main/java/io/hetu/core/seedstore/filebased/FileBasedSeedStore.java +++ b/hetu-seed-store/src/main/java/io/hetu/core/seedstore/filebased/FileBasedSeedStore.java @@ -16,6 +16,7 @@ package io.hetu.core.seedstore.filebased; import io.airlift.log.Logger; +import io.hetu.core.common.util.SecurePathWhiteList; import io.prestosql.spi.filesystem.FileBasedLock; import io.prestosql.spi.filesystem.HetuFileSystemClient; import io.prestosql.spi.seedstore.Seed; @@ -35,6 +36,7 @@ import java.util.concurrent.locks.Lock; import java.util.stream.Collectors; import java.util.stream.Stream; +import static com.google.common.base.Preconditions.checkArgument; import static java.nio.file.StandardOpenOption.CREATE_NEW; /** @@ -68,6 +70,15 @@ public class FileBasedSeedStore this.config = config; seedDir = Paths.get(config.get(FileBasedSeedConstants.SEED_STORE_FILESYSTEM_DIR).trim()); + try { + checkArgument(!seedDir.toString().contains("../"), + "SeedStore directory path must be absolute and at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + checkArgument(SecurePathWhiteList.isSecurePath(seedDir.toString()), + "SeedStore directory path must be at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new IllegalArgumentException("Failed to get secure path list.", e); + } seedFilePath = seedDir.resolve(name).resolve(FileBasedSeedConstants.SEED_FILE_NAME); } diff --git a/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java b/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java index 151b76d37..e9c9972bf 100644 --- a/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java +++ b/presto-hive/src/main/java/io/prestosql/plugin/hive/util/IndexCache.java @@ -91,6 +91,18 @@ public class IndexCache .filter(key -> partitions == null || !partitions.contains(key)) .map(HiveColumnHandle::getName) .map(String::toLowerCase).forEach(column -> { + // security check required before using values in a Path + // e.g. catalog.schema.table or dc.catalog.schema.table + if (!tableFqn.matches("([\\p{Alnum}_]+\\.){2,3}[\\p{Alnum}_]+")) { + LOG.warn("Invalid table name " + tableFqn); + return; + } + + if (!column.matches("[\\p{Alnum}_]+")) { + LOG.warn("Invalid column name " + column); + return; + } + String indexCacheKeyPath = Paths.get(tableFqn, column, pathUri.getPath()).toString(); IndexCacheKey indexCacheKey = new IndexCacheKey(indexCacheKeyPath, lastModifiedTime, "bitmap", "bloom"); // check if cache contains the key diff --git a/presto-hive/src/test/java/io/prestosql/plugin/hive/util/TestIndexCache.java b/presto-hive/src/test/java/io/prestosql/plugin/hive/util/TestIndexCache.java index 4a7a20f2f..c669acea1 100644 --- a/presto-hive/src/test/java/io/prestosql/plugin/hive/util/TestIndexCache.java +++ b/presto-hive/src/test/java/io/prestosql/plugin/hive/util/TestIndexCache.java @@ -59,7 +59,7 @@ public class TestIndexCache String catalog = "test_catalog"; String column = "column_name"; - String table = "table_name"; + String table = "schema_name.table_name"; long testLastModifiedTime = 1; String testPath = "/user/hive/schema.db/table/001.orc"; List testPartitions = Collections.emptyList(); @@ -96,7 +96,7 @@ public class TestIndexCache String catalog = "test_catalog"; String column = "column_name"; - String table = "table_name"; + String table = "schema_name.table_name"; long testLastModifiedTime = 1; String testPath = "/user/hive/schema.db/table/001.orc"; List testPartitions = Collections.emptyList(); @@ -135,7 +135,7 @@ public class TestIndexCache String catalog = "test_catalog"; String column = "column_name"; - String table = "table_name"; + String table = "schema_name.table_name"; long testLastModifiedTime = 1; String testPath = "/user/hive/schema.db/table/001.orc"; List testPartitions = Collections.emptyList(); @@ -179,7 +179,7 @@ public class TestIndexCache String catalog = "test_catalog"; String column = "column_name"; - String table = "table_name"; + String table = "schema_name.table_name"; long testLastModifiedTime = 1; String testPath = "/user/hive/schema.db/table/001.orc"; diff --git a/presto-main/src/main/java/io/prestosql/catalog/AbstractCatalogStore.java b/presto-main/src/main/java/io/prestosql/catalog/AbstractCatalogStore.java index b2737a20f..8904f851f 100644 --- a/presto-main/src/main/java/io/prestosql/catalog/AbstractCatalogStore.java +++ b/presto-main/src/main/java/io/prestosql/catalog/AbstractCatalogStore.java @@ -18,6 +18,7 @@ package io.prestosql.catalog; import com.google.common.io.ByteStreams; import io.airlift.json.JsonCodec; import io.airlift.log.Logger; +import io.hetu.core.common.util.SecurePathWhiteList; import io.prestosql.spi.PrestoException; import io.prestosql.spi.filesystem.FileBasedLock; import io.prestosql.spi.filesystem.HetuFileSystemClient; @@ -36,6 +37,7 @@ import java.util.Set; import java.util.concurrent.locks.Lock; import java.util.stream.Stream; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.Maps.fromProperties; import static io.prestosql.catalog.CatalogFilePath.getCatalogBasePath; @@ -57,6 +59,16 @@ public abstract class AbstractCatalogStore public AbstractCatalogStore(String baseDirectory, HetuFileSystemClient fileSystemClient, int maxFileSizeInBytes) { this.baseDirectory = requireNonNull(baseDirectory, "baseDirectory is null"); + try { + checkArgument(!baseDirectory.contains("../"), + "Catalog directory path must be absolute and at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + checkArgument(SecurePathWhiteList.isSecurePath(baseDirectory), + "Catalog file directory path must at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new IllegalArgumentException("Catalog file path not secure", e); + } + this.maxFileSizeInBytes = requireNonNull(maxFileSizeInBytes, "maxFileSizeInBytes is null"); this.fileSystemClient = requireNonNull(fileSystemClient, "fileSystemClient is null"); if (!fileSystemClient.exists(getCatalogBasePath(baseDirectory))) { diff --git a/presto-main/src/main/java/io/prestosql/catalog/CatalogFilePath.java b/presto-main/src/main/java/io/prestosql/catalog/CatalogFilePath.java index 8f0927e44..76bf98a0c 100644 --- a/presto-main/src/main/java/io/prestosql/catalog/CatalogFilePath.java +++ b/presto-main/src/main/java/io/prestosql/catalog/CatalogFilePath.java @@ -15,9 +15,13 @@ package io.prestosql.catalog; +import io.hetu.core.common.util.SecurePathWhiteList; + +import java.io.IOException; import java.nio.file.Path; import java.nio.file.Paths; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; public final class CatalogFilePath @@ -49,10 +53,21 @@ public final class CatalogFilePath requireNonNull(baseDirectory, "base directory is null"); requireNonNull(catalogName, "catalog name is null"); + try { + checkArgument(!baseDirectory.contains("../"), + "Catalog directory path must be absolute and at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + checkArgument(SecurePathWhiteList.isSecurePath(baseDirectory), + "Catalog file directory path must at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new IllegalArgumentException("Catalog file path not secure", e); + } + // global files directory this.globalDirPath = Paths.get(baseDirectory, "global"); // catalog files directory String catalogBasePath = getCatalogBasePath(baseDirectory).toString(); + this.catalogDirPath = Paths.get(catalogBasePath, catalogName); this.propertiesPath = Paths.get(catalogBasePath, catalogName + ".properties"); this.metadataPath = Paths.get(catalogDirPath.toString(), catalogName + ".metadata"); diff --git a/presto-main/src/main/java/io/prestosql/catalog/CatalogResource.java b/presto-main/src/main/java/io/prestosql/catalog/CatalogResource.java index e9583e56b..db37f1c49 100644 --- a/presto-main/src/main/java/io/prestosql/catalog/CatalogResource.java +++ b/presto-main/src/main/java/io/prestosql/catalog/CatalogResource.java @@ -40,6 +40,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.List; +import static com.google.common.base.Preconditions.checkArgument; import static io.prestosql.catalog.CatalogFileInputStream.CatalogFileType.CATALOG_FILE; import static io.prestosql.catalog.CatalogFileInputStream.CatalogFileType.GLOBAL_FILE; import static io.prestosql.catalog.DynamicCatalogService.badRequest; @@ -177,6 +178,8 @@ public class CatalogResource public Response dropCatalog(@NotNull @PathParam("catalogName") String catalogName, @Context HttpServletRequest servletRequest) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + return service.dropCatalog(catalogName, new HttpRequestSessionContext(servletRequest)); } diff --git a/presto-main/src/main/java/io/prestosql/catalog/DynamicCatalogService.java b/presto-main/src/main/java/io/prestosql/catalog/DynamicCatalogService.java index 45c53b97b..6ef4804db 100644 --- a/presto-main/src/main/java/io/prestosql/catalog/DynamicCatalogService.java +++ b/presto-main/src/main/java/io/prestosql/catalog/DynamicCatalogService.java @@ -31,6 +31,7 @@ import java.io.IOException; import java.util.Set; import java.util.concurrent.locks.Lock; +import static com.google.common.base.Preconditions.checkArgument; import static java.util.Objects.requireNonNull; import static javax.ws.rs.core.MediaType.TEXT_PLAIN_TYPE; import static javax.ws.rs.core.Response.Status.BAD_REQUEST; @@ -96,6 +97,8 @@ public class DynamicCatalogService HttpRequestSessionContext sessionContext) { String catalogName = catalogInfo.getCatalogName(); + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + // check the permission. try { accessControl.checkCanCreateCatalog(sessionContext.getIdentity(), catalogName); @@ -169,6 +172,7 @@ public class DynamicCatalogService HttpRequestSessionContext sessionContext) { String catalogName = catalogInfo.getCatalogName(); + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); // check the permission. try { @@ -233,6 +237,8 @@ public class DynamicCatalogService public synchronized Response dropCatalog(String catalogName, HttpRequestSessionContext sessionContext) { + checkArgument(catalogName.matches("[\\p{Alnum}_]+"), "Invalid catalog name"); + // check the permission. try { accessControl.checkCanDropCatalog(sessionContext.getIdentity(), catalogName); diff --git a/presto-spi/pom.xml b/presto-spi/pom.xml index 012de6213..13b4a0434 100644 --- a/presto-spi/pom.xml +++ b/presto-spi/pom.xml @@ -49,6 +49,11 @@ jol-core + + io.hetu.core + hetu-common + + org.testng diff --git a/presto-spi/src/main/java/io/prestosql/spi/filesystem/FileBasedLock.java b/presto-spi/src/main/java/io/prestosql/spi/filesystem/FileBasedLock.java index b84ff455a..f91c657e5 100644 --- a/presto-spi/src/main/java/io/prestosql/spi/filesystem/FileBasedLock.java +++ b/presto-spi/src/main/java/io/prestosql/spi/filesystem/FileBasedLock.java @@ -16,6 +16,7 @@ package io.prestosql.spi.filesystem; import com.google.common.util.concurrent.UncheckedExecutionException; import io.airlift.log.Logger; +import io.hetu.core.common.util.SecurePathWhiteList; import java.io.FileNotFoundException; import java.io.IOException; @@ -40,6 +41,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; +import static com.google.common.base.Preconditions.checkArgument; import static java.nio.file.StandardOpenOption.CREATE_NEW; /** @@ -92,6 +94,15 @@ public class FileBasedLock String retryIntervalRead = lockProperties.getProperty(LOCK_RETRY_INTERVAL_CONFIG); String refreshRateRead = lockProperties.getProperty(LOCK_REFRESH_RATE_CONFIG); + try { + checkArgument(!lockDir.contains("../"), + "Lock directory path must be absolute and at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + checkArgument(SecurePathWhiteList.isSecurePath(lockDir), + "Lock directory path must be at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new IllegalArgumentException("Failed to get secure path list.", e); + } Path lockFileDir = Paths.get(lockDir); long timeout = (timeoutRead == null) ? DEFAULT_LOCK_FILE_TIMEOUT : Long.parseLong(timeoutRead); long retryInterval = (retryIntervalRead == null) ? DEFAULT_RETRY_INTERVAL : Long.parseLong(retryIntervalRead); @@ -130,6 +141,15 @@ public class FileBasedLock long refreshRate) throws IOException { + try { + checkArgument(!lockFileDir.toString().contains("../"), + "Lock directory path must be absolute and at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + checkArgument(SecurePathWhiteList.isSecurePath(lockFileDir.toString()), + "Lock directory path must be at user workspace " + SecurePathWhiteList.getSecurePathWhiteList().toString()); + } + catch (IOException e) { + throw new IllegalArgumentException("Failed to get secure path list.", e); + } fs.createDirectories(lockFileDir); this.fs = fs; this.uuid = UUID.randomUUID();