Mercurial > hg > mercurial

changeset 1059:cd7011049ea4 Lakhnau-2020.2.x

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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 ");
+  }
 }