[SECURITY-1534] Prevent remote execution by repo URL

SECURITY-1534 reports that user input in the repository URL field is not
validated sufficiently. A carefully crafted value in the URL field can
allow a user with Job administration permissions to execute an arbitrary
program on the Jenkins master.

Sanity check the values passed as repository URL to the ls-remote and
fetch commands so that user entered data cannot execute arbitrary programs
on the Jenkins master.

Use -Dorg.jenkinsci.plugins.gitclient.CliGitAPIImpl.checkRemoteURL=false
to disable URL checking.

src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java

@@ -369,6 +369,67 @@ public List<IndexEntry> getSubmodules( String treeIsh ) throws GitException, Int
         return submodules;
     }
 
+    /**
+     * Constant which disables safety check of remote URL.
+     *
+     * <code>CHECK_REMOTE_URL=Boolean.valueOf(System.getProperty(CliGitAPIImpl.class.getName() + ".checkRemoteURL", "true"))</code>.
+     *
+     * Refuse unsafe git URL's, including URL's that start with '-'
+     * and URL's that contain space characters.
+     *
+     * Use '-Dorg.jenkinsci.plugins.gitclient.CliGitAPIImpl.checkRemoteURL=false'
+     * to prevent check of remote URL.
+     */
+    private static final boolean CHECK_REMOTE_URL = Boolean.valueOf(System.getProperty(CliGitAPIImpl.class.getName() + ".checkRemoteURL", "true"));
+
+    /**
+     * SECURITY-1534 found that arguments
+     * added to a git URL from the user interface can allow a user
+     * with job creation permissions to execute an arbitrary program
+     * on the git server if the git server is configured to allow
+     * custom pack programs. Reject a URL if it includes invalid
+     * content.
+     */
+    private void addCheckedRemoteUrl(@NonNull ArgumentListBuilder args, @NonNull String url) {
+        String trimmedUrl = url.trim();
+        /* Don't check for invalid args if URL starts with known good cases.
+         * Known good cases include:
+         * '/' - Unix local file
+         * 'C:' - Windows local file
+         * 'file:', 'git:', 'http:', 'https:', 'ssh:', and 'git@' - known protocols
+         */
+        if (CHECK_REMOTE_URL
+                && !trimmedUrl.startsWith("/")
+                && !trimmedUrl.startsWith("\\\\")
+                && !trimmedUrl.startsWith("file:")
+                && !trimmedUrl.startsWith("git:")
+                && !trimmedUrl.startsWith("git@")
+                && !trimmedUrl.startsWith("http:")
+                && !trimmedUrl.startsWith("https:")
+                && !trimmedUrl.startsWith("ssh:")
+                && !trimmedUrl.matches("^[A-Za-z]:.+")) {
+            /* Not one of the known good cases, check if this could be a bad case
+             * Bad cases include:
+             * '-' - starts with 'dash' as possible argument
+             * '`' - includes backquote in the string (command execution)
+             * ' ' - includes a space (not guaranteed a threat, but threat risk increases)
+             */
+            if (trimmedUrl.startsWith("-")
+                    || trimmedUrl.contains("`")
+                    || trimmedUrl.contains("--upload-pack=")
+                    || trimmedUrl.matches(".*\\s+.*")) {
+                throw new GitException("Invalid remote URL: " + url);
+            }
+        }
+        // Mark the end of options for git versions that support it.
+        // Tells command line git that later arguments are operands, not options.
+        // See POSIX 1-2017 guideline 10.
+        if (isAtLeastVersion(2, 8, 0, 0)) {
+            args.add("--"); // SECURITY-1534 - end of options, causes tests to fail with git 2.7.4 and older
+        }
+        args.add(trimmedUrl);
+    }
+
     /**
      * fetch_.
      *
@@ -497,7 +558,7 @@ public void fetch(String remoteName, RefSpec... refspec) throws GitException, In
         String url = getRemoteUrl(remoteName);
         if (url == null)
             throw new GitException("remote." + remoteName + ".url not defined");
-        args.add(url);
+        addCheckedRemoteUrl(args, url);
         if (refspec != null && refspec.length > 0)
             for (RefSpec rs: refspec)
                 if (rs != null)
@@ -2770,7 +2831,10 @@ public Set<String> getRemoteTagNames(String tagPattern) throws GitException {
         try {
             ArgumentListBuilder args = new ArgumentListBuilder();
             args.add("ls-remote", "--tags");
-            args.add(getRemoteUrl("origin"));
+            String remoteUrl = getRemoteUrl("origin");
+            if (remoteUrl != null) {
+                addCheckedRemoteUrl(args, remoteUrl);
+            }
             if (tagPattern != null)
                 args.add(tagPattern);
             String result = launchCommandIn(args, workspace);
@@ -2872,7 +2936,7 @@ public Set<String> getRefNames(String refPrefix) throws GitException, Interrupte
     public Map<String, ObjectId> getHeadRev(String url) throws GitException, InterruptedException {
         ArgumentListBuilder args = new ArgumentListBuilder("ls-remote");
         args.add("-h");
-        args.add(url);
+        addCheckedRemoteUrl(args, url);
 
         StandardCredentials cred = credentials.get(url);
         if (cred == null) cred = defaultCredentials;
@@ -2902,7 +2966,8 @@ public ObjectId getHeadRev(String url, String branchSpec) throws GitException, I
         StandardCredentials cred = credentials.get(url);
         if (cred == null) cred = defaultCredentials;
 
-        args.add(url);
+        addCheckedRemoteUrl(args, url);
+
         if (branchName.startsWith("refs/tags/")) {
             args.add(branchName+"^{}"); // JENKINS-23299 - tag SHA1 needs to be converted to commit SHA1
         } else {
@@ -2922,7 +2987,7 @@ public Map<String, ObjectId> getRemoteReferences(String url, String pattern, boo
         if (tagsOnly) {
             args.add("-t");
         }
-        args.add(url);
+        addCheckedRemoteUrl(args, url);
         if (pattern != null) {
             args.add(pattern);
         }
@@ -2964,7 +3029,7 @@ public Map<String, String> getRemoteSymbolicReferences(String url, String patter
             // https://github.com/git/git/blob/afd6726309/Documentation/RelNotes/2.8.0.txt#L72-L73
             ArgumentListBuilder args = new ArgumentListBuilder("ls-remote");
             args.add("--symref");
-            args.add(url);
+            addCheckedRemoteUrl(args, url);
             if (pattern != null) {
                 args.add(pattern);
             }
@@ -3013,7 +3078,8 @@ public void push(RemoteConfig repository, String refspec) throws GitException, I
         StandardCredentials cred = credentials.get(url);
         if (cred == null) cred = defaultCredentials;
 
-        args.add("push", url);
+        args.add("push");
+        addCheckedRemoteUrl(args, url);
 
         if (refspec != null)
             args.add(refspec);