Mercurial > hg > mercurial
changeset 1059:cd7011049ea4 Lakhnau-2020.2.x
TW-70541 prohibited urls, branch and tag names containing --cwd substring because it can be user for RCE attack
(grafted from 8e5475e8adbd9899f7a56c5ba09c3043e2192245)
author | victory.bedrosova |
---|---|
date | Thu, 11 Mar 2021 14:01:08 +0100 |
parents | e77ffd8ebfee |
children | 24b84c5028bf |
files | mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/HgVcsRoot.java mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialVcsSupport.java mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialUrlSupportTest.java |
diffstat | 3 files changed, 23 insertions(+), 9 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/HgVcsRoot.java Thu Mar 11 12:39:28 2021 +0300 +++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/HgVcsRoot.java Thu Mar 11 14:01:08 2021 +0100 @@ -33,6 +33,7 @@ */ public class HgVcsRoot { public static final String DEFAULT_BRANCH_NAME = "default"; + public static final String[] PROHIBITED_OPTIONS = {"--config", "--debug", "--cwd"}; private final Map<String, String> myVcsRootProperties; private final String myRepository; @@ -279,18 +280,16 @@ return false; } - // prevent RCE, see TW-70054 + // prevent RCE, see TW-70054, TW-70541 @Nullable private String getValidatedProperty(@NotNull String name) { final String param = getProperty(name); if (StringUtil.isEmpty(param)) return null; - final String format = "Parameter" + name + " is not allowed to contain %s substring for security reasons"; - if (param.contains("--config")) { - throw new IllegalArgumentException(String.format(format, "--config")); - } - if (param.contains("--debug")) { - throw new IllegalArgumentException(String.format(format, "--debug")); + for (String s : PROHIBITED_OPTIONS) { + if (param.contains(s)) { + throw new IllegalArgumentException(String.format("Parameter" + name + " is not allowed to contain %s substring for security reasons", s)); + } } return param; }
--- a/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialVcsSupport.java Thu Mar 11 12:39:28 2021 +0300 +++ b/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialVcsSupport.java Thu Mar 11 14:01:08 2021 +0100 @@ -753,8 +753,12 @@ // Carriage return (ASCII 13, '\r') // Newline (ASCII 10, '\n') // all these characters will be replaced with _ (underscore) - // we also replace --config and --debug substrings because they may be a sign of RCE, see TW-70054 - return label.replace(':', '_').replace('\r', '_').replace('\n', '_').replace("--config", "_").replace("--debug", "_"); + // we also replace --config, --debug and --cwd substrings because they may be a sign of RCE, see TW-70054, TW-70541 + String result = label.replace(':', '_').replace('\r', '_').replace('\n', '_'); + for (String o : HgVcsRoot.PROHIBITED_OPTIONS) { + result = result.replace(o, "_"); + } + return result; } public File getWorkingDir(HgVcsRoot root) {
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialUrlSupportTest.java Thu Mar 11 12:39:28 2021 +0300 +++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialUrlSupportTest.java Thu Mar 11 14:01:08 2021 +0100 @@ -88,4 +88,15 @@ } fail("Expection not thrown "); } + + @TestFor(issues = "TW-70541") + public void exception_on_command_injection_cwd() throws VcsException { + try { + myUrlSupport.convertToVcsRootProperties(new VcsUrl("--cwd=/home/user/.BuildServer/system/artifacts/Test/bad_artifact/411/")); + } catch (IllegalArgumentException e) { + // expected + return; + } + fail("Expection not thrown "); + } }