From c7be8fc501e318589f1346c3e7bece9359ef6fbb Mon Sep 17 00:00:00 2001 From: Jonathan T-Delorme Date: Wed, 30 Sep 2026 11:41:54 -0400 Subject: [PATCH 1/2] Fix command-line argument injection in Diff action A crafted filename such as "payload --extcmd=wscript.exe" was interpolated unquoted into the git difftool argument string. Since ProcessStartInfo.Arguments is parsed as a whitespace-delimited command line, the filename split into two arguments, letting git interpret --extcmd (or --ext-diff/-x) as a real option and execute an attacker-chosen program with the victim's privileges when they clicked Diff on a crafted repository. Fix by quoting the path arguments and inserting -- before them so git always treats them as positional paths, never as options. Co-Authored-By: Claude Sonnet 5 --- GitTimelapseView/Actions/DiffFileChangeAction.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/GitTimelapseView/Actions/DiffFileChangeAction.cs b/GitTimelapseView/Actions/DiffFileChangeAction.cs index 28d1433..8662bbe 100644 --- a/GitTimelapseView/Actions/DiffFileChangeAction.cs +++ b/GitTimelapseView/Actions/DiffFileChangeAction.cs @@ -34,7 +34,9 @@ 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 = path.Equals(oldPath, StringComparison.OrdinalIgnoreCase) + ? $"difftool -y {_commitId}^ {_commitId} -- \"{path}\"" + : $"difftool -y {_commitId}^ {_commitId} -- \"{oldPath}\" \"{path}\""; GitHelpers.RunGitCommand(_fileChange.Commit.FileHistory.GitRootPath, args, context, errorMessage); return Task.CompletedTask; From 487ca7b6538f712bc1a83227c51a1be4d44e38a6 Mon Sep 17 00:00:00 2001 From: Jonathan T-Delorme Date: Wed, 30 Sep 2026 11:43:40 -0400 Subject: [PATCH 2/2] Add regression tests for difftool argument injection fix Extract the difftool argument-building logic into GitHelpers.BuildDiffToolArguments so it is unit-testable without pulling WPF dependencies into the test project, and add tests that exercise the crafted-filename payload from the report (e.g. "payload --extcmd=wscript.exe") to confirm it is always kept as a single quoted, positional path after '--' and never parsed as a git option. Co-Authored-By: Claude Sonnet 5 --- GitTimelapseView.Core/Common/GitHelpers.cs | 13 +++++ GitTimelapseView.Tests/GitHelpersTests.cs | 56 +++++++++++++++++++ .../Actions/DiffFileChangeAction.cs | 4 +- 3 files changed, 70 insertions(+), 3 deletions(-) create mode 100644 GitTimelapseView.Tests/GitHelpersTests.cs diff --git a/GitTimelapseView.Core/Common/GitHelpers.cs b/GitTimelapseView.Core/Common/GitHelpers.cs index 7f42137..c59470a 100644 --- a/GitTimelapseView.Core/Common/GitHelpers.cs +++ b/GitTimelapseView.Core/Common/GitHelpers.cs @@ -48,6 +48,19 @@ public static string RunGitCommand(string gitRootPath, string args, ILogger logg return gitProcess.StandardOutput.ReadToEnd().Trim(); } + /// + /// 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. + /// + 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)) diff --git a/GitTimelapseView.Tests/GitHelpersTests.cs b/GitTimelapseView.Tests/GitHelpersTests.cs new file mode 100644 index 0000000..182dec7 --- /dev/null +++ b/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\"")); + } + } +} diff --git a/GitTimelapseView/Actions/DiffFileChangeAction.cs b/GitTimelapseView/Actions/DiffFileChangeAction.cs index 8662bbe..8b081fc 100644 --- a/GitTimelapseView/Actions/DiffFileChangeAction.cs +++ b/GitTimelapseView/Actions/DiffFileChangeAction.cs @@ -34,9 +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}\"" - : $"difftool -y {_commitId}^ {_commitId} -- \"{oldPath}\" \"{path}\""; + var args = GitHelpers.BuildDiffToolArguments(_commitId, oldPath, path); GitHelpers.RunGitCommand(_fileChange.Commit.FileHistory.GitRootPath, args, context, errorMessage); return Task.CompletedTask;