This is an automated email from the ASF dual-hosted git repository. shaofengshi pushed a commit to branch 3.0.x in repository https://gitbox.apache.org/repos/asf/kylin.git
commit c5dda34f6c53514ff32584f5c5b9fd1a25c38b7d Author: XiaoxiangYu <hit_la...@126.com> AuthorDate: Tue Mar 17 07:16:31 2020 +0800 KYLIN-4426 Refine CliCommandExecutor --- .../kylin/common/util/CliCommandExecutor.java | 36 +++++++++++++++++-- .../kylin/common/util/CliCommandExecutorTest.java | 42 ++++++++++++++++++++++ .../org/apache/kylin/rest/service/CubeService.java | 9 +++-- 3 files changed, 82 insertions(+), 5 deletions(-) diff --git a/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java b/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java index 6b8cf76..eda3c5e 100644 --- a/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java +++ b/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java @@ -6,15 +6,15 @@ * 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.kylin.common.util; @@ -163,4 +163,34 @@ public class CliCommandExecutor { } } + public static final String COMMAND_INJECT_REX = "[ &`>|{}()$;\\-#~!+*”\\\\]+"; + + /** + * <pre> + * Check parameter for preventing command injection, replace illegal character into empty character. + * + * Note: + * 1. Whitespace is also refused because parameter is a single word, should not contains it + * 2. Some character may be illegal but still be accepted because commandParameter maybe a URI/path expression, + * you may check "Character part" in https://docs.oracle.com/javase/8/docs/api/java/net/URI.html, + * here is the character which is not banned. + * + * 1. dot . + * 2. slash / + * 3. colon : + * 4. equal = + * 5. ? + * 6. @ + * 7. bracket [] + * 8. comma , + * 9. % + * </pre> + */ + public static String checkParameter(String commandParameter) { + String repaired = commandParameter.replaceAll(COMMAND_INJECT_REX, ""); + if (repaired.length() != commandParameter.length()) { + logger.info("Detected illegal character in command {}, replace it to {}.", commandParameter, repaired); + } + return repaired; + } } diff --git a/core-common/src/test/java/org/apache/kylin/common/util/CliCommandExecutorTest.java b/core-common/src/test/java/org/apache/kylin/common/util/CliCommandExecutorTest.java new file mode 100644 index 0000000..b088e02 --- /dev/null +++ b/core-common/src/test/java/org/apache/kylin/common/util/CliCommandExecutorTest.java @@ -0,0 +1,42 @@ +/* + * 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.kylin.common.util; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; + +public class CliCommandExecutorTest { + + @Test + public void testCmd() { + String[][] commands = { + {"nslookup unknown.com &", "nslookupunknown.com"}, + {"cat `whoami`", "catwhoami"}, + {"whoami > /var/www/static/whoami.txt", "whoami/var/www/static/whoami.txt"}, + {"c1 || c2# || c3 || *c4\\", "c1c2c3c4"}, + {"c1 &&", "c1"}, + {"c1 + > c2 [p1]%", "c1c2[p1]%"}, + {"c1 | ${c2}", "c1c2"}, + }; + + for (String[] pair : commands) { + assertEquals(pair[1], CliCommandExecutor.checkParameter(pair[0])); + } + } +} diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java b/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java index 11b06a1..0c5d417 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java @@ -1103,8 +1103,13 @@ public class CubeService extends BasicService implements InitializingBean { "Destination configuration should not be empty."); String stringBuilder = ("%s/bin/kylin.sh org.apache.kylin.tool.CubeMigrationCLI %s %s %s %s %s %s true true"); - String cmd = String.format(Locale.ROOT, stringBuilder, KylinConfig.getKylinHome(), srcCfgUri, dstCfgUri, - cube.getName(), projectName, config.isAutoMigrateCubeCopyAcl(), config.isAutoMigrateCubePurge()); + String cmd = String.format(Locale.ROOT, stringBuilder, KylinConfig.getKylinHome(), + CliCommandExecutor.checkParameter(srcCfgUri), + CliCommandExecutor.checkParameter(dstCfgUri), + cube.getName(), + CliCommandExecutor.checkParameter(projectName), + config.isAutoMigrateCubeCopyAcl(), + config.isAutoMigrateCubePurge()); logger.info("One click migration cmd: " + cmd);