Fix command-line argument injection in Diff action (CVSS 8.6)

GitTimelapseView.Core/Common/GitHelpers.cs

@@ -48,6 +48,19 @@ public static string RunGitCommand(string gitRootPath, string args, ILogger logg
             return gitProcess.StandardOutput.ReadToEnd().Trim();
         }
 
+        /// <summary>
+        /// Builds the argument string for `git difftool` between a commit and its parent for one or two paths.
+        /// Paths are quoted and preceded by `--` so that a crafted file/folder name (e.g. containing
+        /// "--extcmd=..." or another option-like token) is always treated by git as a literal path, never
+        /// as a command-line option.
+        /// </summary>
+        public static string BuildDiffToolArguments(string commitId, string oldPath, string path)
+        {
+            return oldPath.Equals(path, StringComparison.OrdinalIgnoreCase)
+                ? $"difftool -y {commitId}^ {commitId} -- \"{path}\""
+                : $"difftool -y {commitId}^ {commitId} -- \"{oldPath}\" \"{path}\"";
+        }
+
         public static string? GetRemotePlatform(string remoteUrl)
         {
             if (remoteUrl.Contains("github.com", StringComparison.OrdinalIgnoreCase))

GitTimelapseView.Tests/GitHelpersTests.cs

@@ -0,0 +1,56 @@
+// Copyright (c) Ubisoft. All Rights Reserved.
+// Licensed under the Apache License, Version 2.0.
+
+using GitTimelapseView.Core.Common;
+using NUnit.Framework;
+
+namespace GitTimelapseView.Tests
+{
+    [TestFixture]
+    public class GitHelpersTests
+    {
+        [Test]
+        public void BuildDiffToolArguments_SamePath_QuotesPathAndUsesDoubleDashSeparator()
+        {
+            var args = GitHelpers.BuildDiffToolArguments("abc123", @"C:\repo\file.txt", @"C:\repo\file.txt");
+
+            Assert.That(args, Is.EqualTo("difftool -y abc123^ abc123 -- \"C:\\repo\\file.txt\""));
+        }
+
+        [Test]
+        public void BuildDiffToolArguments_DifferentPaths_QuotesBothPathsAndUsesDoubleDashSeparator()
+        {
+            var args = GitHelpers.BuildDiffToolArguments("abc123", @"C:\repo\old.txt", @"C:\repo\new.txt");
+
+            Assert.That(args, Is.EqualTo("difftool -y abc123^ abc123 -- \"C:\\repo\\old.txt\" \"C:\\repo\\new.txt\""));
+        }
+
+        [Test]
+        public void BuildDiffToolArguments_FileNameLooksLikeAnOption_IsKeptAsALiteralQuotedPathAfterDoubleDash()
+        {
+            // Regression test for a crafted filename such as "payload --extcmd=wscript.exe" being
+            // interpreted as a git command-line option instead of a literal path.
+            var maliciousPath = @"C:\repo\payload --extcmd=wscript.exe";
+
+            var args = GitHelpers.BuildDiffToolArguments("abc123", maliciousPath, maliciousPath);
+
+            // The whole malicious name must stay inside a single quoted token, placed after "--",
+            // so git can never parse "--extcmd=..." as a separate, option-like argument.
+            Assert.That(args, Is.EqualTo("difftool -y abc123^ abc123 -- \"C:\\repo\\payload --extcmd=wscript.exe\""));
+            Assert.That(args, Does.Contain("-- \""), "Paths must be preceded by the '--' end-of-options marker.");
+            Assert.That(args.IndexOf("--extcmd", StringComparison.Ordinal), Is.GreaterThan(args.IndexOf("-- \"", StringComparison.Ordinal)),
+                "The attacker-controlled '--extcmd' text must appear inside the quoted path, after the '--' separator, not as a standalone option.");
+        }
+
+        [Test]
+        public void BuildDiffToolArguments_OldPathLooksLikeAnOption_IsKeptAsALiteralQuotedPath()
+        {
+            var maliciousOldPath = @"C:\repo\payload --extcmd=wscript.exe";
+            var newPath = @"C:\repo\payload.txt";
+
+            var args = GitHelpers.BuildDiffToolArguments("abc123", maliciousOldPath, newPath);
+
+            Assert.That(args, Is.EqualTo("difftool -y abc123^ abc123 -- \"C:\\repo\\payload --extcmd=wscript.exe\" \"C:\\repo\\payload.txt\""));
+        }
+    }
+}

GitTimelapseView/Actions/DiffFileChangeAction.cs

@@ -34,7 +34,7 @@ public override Task ExecuteAsync(IActionContext context)
             context.LogInformation($"Diffing '{path}' with commit '{_commitId}'");
 
             var errorMessage = $"Could not launch difftool on {path} for commit {_commitId}";
-            var args = path.Equals(oldPath, StringComparison.OrdinalIgnoreCase) ? $"difftool -y {_commitId}^ {_commitId} {path}".Trim(' ') : $"difftool -y {_commitId}^ {_commitId} {oldPath} {path}".Trim(' ');
+            var args = GitHelpers.BuildDiffToolArguments(_commitId, oldPath, path);
             GitHelpers.RunGitCommand(_fileChange.Commit.FileHistory.GitRootPath, args, context, errorMessage);
 
             return Task.CompletedTask;