changeset 1054:b01c7d60afb2 release-92072

TW-70054
author victory.bedrosova
date Fri, 12 Feb 2021 16:27:47 +0100
parents 4c2548700627
children efc2b5c766fe
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	Wed Jan 20 10:21:31 2021 +0100
+++ 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	Wed Jan 20 10:21:31 2021 +0100
+++ 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	Wed Jan 20 10:21:31 2021 +0100
+++ 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 ");
+  }
 }