Mercurial > hg > mercurial
changeset 1055:516413ed5087 Lakhnau-2020.2.x release-86002
TW-70054
(grafted from b01c7d60afb2aeb80c115caf0e2b45078d8a3047)
author | victory.bedrosova |
---|---|
date | Fri, 12 Feb 2021 16:27:47 +0100 |
parents | 874643784687 |
children | e77ffd8ebfee |
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, 32 insertions(+), 3 deletions(-) [+] |
line wrap: on
line diff
--- a/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/HgVcsRoot.java Fri Jan 29 13:16:23 2021 +0300 +++ b/mercurial-common/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/command/HgVcsRoot.java Fri Feb 12 16:27:47 2021 +0100 @@ -57,9 +57,9 @@ public HgVcsRoot(@NotNull Map<String, String> vcsRootProperties) { myVcsRootProperties = vcsRootProperties; - myRepository = getProperty(Constants.REPOSITORY_PROP); + myRepository = getValidatedProperty(Constants.REPOSITORY_PROP); myHgCommandPath = getProperty(Constants.HG_COMMAND_PATH_PROP); - myBranchName = getProperty(Constants.BRANCH_NAME_PROP); + myBranchName = getValidatedProperty(Constants.BRANCH_NAME_PROP); myCustomClonePath = readCustomClonePath(); myUserForTag = getProperty(Constants.USER_FOR_TAG); myAuthSettings = new AuthSettings(getProperty(Constants.USERNAME), getProperty(Constants.PASSWORD)); @@ -278,4 +278,20 @@ } return false; } + + // prevent RCE, see TW-70054 + @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")); + } + return param; + } }
--- a/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialVcsSupport.java Fri Jan 29 13:16:23 2021 +0300 +++ b/mercurial-server/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialVcsSupport.java Fri Feb 12 16:27:47 2021 +0100 @@ -753,7 +753,8 @@ // Carriage return (ASCII 13, '\r') // Newline (ASCII 10, '\n') // all these characters will be replaced with _ (underscore) - return label.replace(':', '_').replace('\r', '_').replace('\n', '_'); + // 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", "_"); } public File getWorkingDir(HgVcsRoot root) {
--- a/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialUrlSupportTest.java Fri Jan 29 13:16:23 2021 +0300 +++ b/mercurial-tests/src/jetbrains/buildServer/buildTriggers/vcs/mercurial/MercurialUrlSupportTest.java Fri Feb 12 16:27:47 2021 +0100 @@ -16,6 +16,7 @@ package jetbrains.buildServer.buildTriggers.vcs.mercurial; +import jetbrains.buildServer.util.TestFor; import jetbrains.buildServer.vcs.Credentials; import jetbrains.buildServer.vcs.VcsException; import jetbrains.buildServer.vcs.VcsUrl; @@ -76,4 +77,15 @@ Map<String, String> props = myUrlSupport.convertToVcsRootProperties(url); assertNull(props); } + + @TestFor(issues = "TW-70054") + public void exception_on_command_injection() throws VcsException { + try { + myUrlSupport.convertToVcsRootProperties(new VcsUrl("--config=hooks.pre-identify=whoami>/tmp/JRNJRN")); + } catch (IllegalArgumentException e) { + // expected + return; + } + fail("Expection not thrown "); + } }