SONAR-16415 Prevent argument injection on `git blame` native command

diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java
index 10733386574877cac7227e7fccaeff587f1c2a03..4439e71b4aa4a664a9c6e6120838b1aa346a466a 100644 (file)
--- a/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java
+++ b/sonar-scanner-engine/src/main/java/org/sonar/scm/git/GitBlameCommand.java
@@ -27,8 +27,11 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
+import org.apache.commons.lang.math.NumberUtils;
 import org.sonar.api.batch.scm.BlameLine;
 import org.sonar.api.utils.System2;
+import org.sonar.api.utils.Version;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -42,11 +45,16 @@ public class GitBlameCommand {
   private static final String COMMITTER_TIME = "committer-time ";
   private static final String COMMITTER_MAIL = "committer-mail ";
 
+  private static final String MINIMUM_REQUIRED_GIT_VERSION = "2.24.0";
   private static final String DEFAULT_GIT_COMMAND = "git";
   private static final String BLAME_COMMAND = "blame";
   private static final String BLAME_LINE_PORCELAIN_FLAG = "--line-porcelain";
+  private static final String END_OF_OPTIONS_FLAG = "--end-of-options";
   private static final String IGNORE_WHITESPACES = "-w";
 
+  private static final Pattern whitespaceRegex = Pattern.compile("\\s+");
+  private static final Pattern semanticVersionDelimiter = Pattern.compile("\\.");
+
   private final System2 system;
   private final ProcessWrapperFactory processWrapperFactory;
   private String gitCommand;
@@ -73,7 +81,7 @@ public class GitBlameCommand {
       this.gitCommand = locateDefaultGit();
       MutableString stdOut = new MutableString();
       this.processWrapperFactory.create(null, l -> stdOut.string = l, gitCommand, "--version").execute();
-      return stdOut.string != null && stdOut.string.startsWith("git version");
+      return stdOut.string != null && stdOut.string.startsWith("git version") && isCompatibleGitVersion(stdOut.string);
     } catch (Exception e) {
       LOG.debug("Failed to find git native client", e);
       return false;
@@ -110,7 +118,7 @@ public class GitBlameCommand {
   public List<BlameLine> blame(Path baseDir, String fileName) throws Exception {
     BlameOutputProcessor outputProcessor = new BlameOutputProcessor();
     try {
-      this.processWrapperFactory.create(baseDir, outputProcessor::process, gitCommand, BLAME_COMMAND, BLAME_LINE_PORCELAIN_FLAG, IGNORE_WHITESPACES, fileName)
+      this.processWrapperFactory.create(baseDir, outputProcessor::process, gitCommand, BLAME_COMMAND, BLAME_LINE_PORCELAIN_FLAG, IGNORE_WHITESPACES, END_OF_OPTIONS_FLAG, fileName)
         .execute();
     } catch (UncommittedLineException e) {
       LOG.debug("Unable to blame file '{}' - it has uncommitted changes", fileName);
@@ -166,6 +174,26 @@ public class GitBlameCommand {
     }
   }
 
+  private static boolean isCompatibleGitVersion(String gitVersionCommandOutput) {
+    // Due to the danger of argument injection on git blame the use of `--end-of-options` flag is necessary
+    // The flag is available only on git versions >= 2.24.0
+    String gitVersion = whitespaceRegex
+      .splitAsStream(gitVersionCommandOutput)
+      .skip(2)
+      .findFirst()
+      .orElse("");
+
+    String formattedGitVersion = formatGitSemanticVersion(gitVersion);
+    return Version.parse(formattedGitVersion).isGreaterThanOrEqual(Version.parse(MINIMUM_REQUIRED_GIT_VERSION));
+  }
+
+  private static String formatGitSemanticVersion(String version) {
+    return semanticVersionDelimiter
+      .splitAsStream(version)
+      .takeWhile(NumberUtils::isNumber)
+      .collect(Collectors.joining("."));
+  }
+
   private static class MutableString {
     String string;
   }

diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java
index 328383c235c9723cd87b711a7c58a6caf4ca2c12..b3e428c17156e4980a4c3f976d2d8a060fe836c4 100644 (file)
--- a/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java
+++ b/sonar-scanner-engine/src/test/java/org/sonar/scm/git/GitBlameCommandTest.java
@@ -28,6 +28,7 @@ import java.util.Date;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.function.Consumer;
+import java.util.stream.Stream;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.junit.Before;
@@ -152,6 +153,41 @@ public class GitBlameCommandTest {
     assertThat(blameCommand.checkIfEnabled()).isFalse();
   }
 
+  @Test
+  public void git_should_not_be_enabled_if_version_command_is_not_found() {
+    ProcessWrapperFactory mockedCmd = mockGitVersionCommand("error: unknown option `version'");
+    GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, mockedCmd);
+    assertThat(blameCommand.checkIfEnabled()).isFalse();
+  }
+
+  @Test
+  public void git_should_not_be_enabled_if_version_command_does_not_return_string_output() {
+    ProcessWrapperFactory mockedCmd = mockGitVersionCommand(null);
+    GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, mockedCmd);
+    assertThat(blameCommand.checkIfEnabled()).isFalse();
+  }
+
+  @Test
+  public void git_should_be_enabled_if_version_is_equal_or_greater_than_required_minimum() {
+    Stream.of(
+      "git version 2.24.0",
+      "git version 2.25.2.1",
+      "git version 2.24.1.1.windows.2",
+      "git version 2.25.1.msysgit.2"
+    ).forEach(output -> {
+      ProcessWrapperFactory mockedCmd = mockGitVersionCommand(output);
+      GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, mockedCmd);
+      assertThat(blameCommand.checkIfEnabled()).isTrue();
+    });
+  }
+
+  @Test
+  public void git_should_not_be_enabled_if_version_is_less_than_required_minimum() {
+    ProcessWrapperFactory mockFactory = mockGitVersionCommand("git version 1.9.0");
+    GitBlameCommand blameCommand = new GitBlameCommand(System2.INSTANCE, mockFactory);
+    assertThat(blameCommand.checkIfEnabled()).isFalse();
+  }
+
   @Test
   public void throw_exception_if_command_fails() throws Exception {
     Path baseDir = temp.newFolder().toPath();
@@ -247,4 +283,17 @@ public class GitBlameCommandTest {
     // This is needed for Windows, otherwise the created File point to invalid (shortened by Windows) temp folder path
     return temp.newFolder().toPath().toRealPath(LinkOption.NOFOLLOW_LINKS).toFile();
   }
+
+  private ProcessWrapperFactory mockGitVersionCommand(String commandOutput) {
+    ProcessWrapperFactory mockFactory = mock(ProcessWrapperFactory.class);
+    ProcessWrapper mockProcess = mock(ProcessWrapper.class);
+
+    when(mockFactory.create(isNull(), any(), eq("git"), eq("--version"))).then(invocation -> {
+      var argument = (Consumer<String>) invocation.getArgument(1);
+      argument.accept(commandOutput);
+      return mockProcess;
+    });
+
+    return mockFactory;
+  }
 }