Skip to content

Commit 61c0388

Browse files
mjcheethammjcheetham
committed
Merged PR 585167: [Security] Enumerate the PATH environment variable to locate programs on Windows
Enumerate the PATH environment variable to manually locate executables/programs rather than shell out to where.exe on Windows locate these for us. The where.exe utility first checks the current working directory before the PATH for the executable, which we do not want for various security reasons. The which utility on UNIX/POSIX systems does _NOT_ include the current working directory in the search; only the PATH, which is exactly what we want - leave this in place on those platforms. Additionally for locating `git(.exe)` we first check in the `GIT_EXEC_PATH` directory for the executable, if that environment variable was set.
2 parents c3a543a + d5e5cfe commit 61c0388

File tree

9 files changed

+141
-45
lines changed

9 files changed

+141
-45
lines changed

‎src/shared/Microsoft.Git.CredentialManager.Tests/EnvironmentTests.cs

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
using System.Collections.Generic;
2+
using Microsoft.Git.CredentialManager.Interop.Windows;
3+
using Microsoft.Git.CredentialManager.Tests.Objects;
4+
using Xunit;
5+
6+
namespace Microsoft.Git.CredentialManager.Tests
7+
{
8+
public class EnvironmentTests
9+
{
10+
[PlatformFact(Platforms.Windows)]
11+
public void WindowsEnvironment_TryLocateExecutable_NotExists_ReturnFalse()
12+
{
13+
string pathVar = @"C:\Users\john.doe\bin;C:\Windows\system32;C:\Windows";
14+
string execName = "foo.exe";
15+
var fs = new TestFileSystem();
16+
var envars = new Dictionary<string, string> {["PATH"] = pathVar};
17+
var env = new WindowsEnvironment(fs, envars);
18+
19+
bool actualResult = env.TryLocateExecutable(execName, out string actualPath);
20+
21+
Assert.False(actualResult);
22+
Assert.Null(actualPath);
23+
}
24+
25+
[PlatformFact(Platforms.Windows)]
26+
public void WindowsEnvironment_TryLocateExecutable_Windows_Exists_ReturnTrueAndPath()
27+
{
28+
string pathVar = @"C:\Users\john.doe\bin;C:\Windows\system32;C:\Windows";
29+
string execName = "foo.exe";
30+
string expectedPath = @"C:\Windows\system32\foo.exe";
31+
var fs = new TestFileSystem
32+
{
33+
Files = new Dictionary<string, byte[]>
34+
{
35+
[@"C:\Windows\system32\foo.exe"] = new byte[0],
36+
}
37+
};
38+
var envars = new Dictionary<string, string> {["PATH"] = pathVar};
39+
var env = new WindowsEnvironment(fs, envars);
40+
41+
bool actualResult = env.TryLocateExecutable(execName, out string actualPath);
42+
43+
Assert.True(actualResult);
44+
Assert.Equal(expectedPath, actualPath);
45+
}
46+
47+
[PlatformFact(Platforms.Windows)]
48+
public void WindowsEnvironment_TryLocateExecutable_Windows_ExistsMultiple_ReturnTrueAndFirstPath()
49+
{
50+
string pathVar = @"C:\Users\john.doe\bin;C:\Windows\system32;C:\Windows";
51+
string execName = "foo.exe";
52+
string expectedPath = @"C:\Users\john.doe\bin\foo.exe";
53+
var fs = new TestFileSystem
54+
{
55+
Files = new Dictionary<string, byte[]>
56+
{
57+
[@"C:\Users\john.doe\bin\foo.exe"] = new byte[0],
58+
[@"C:\Windows\system32\foo.exe"] = new byte[0],
59+
[@"C:\Windows\foo.exe"] = new byte[0],
60+
}
61+
};
62+
var envars = new Dictionary<string, string> {["PATH"] = pathVar};
63+
var env = new WindowsEnvironment(fs, envars);
64+
65+
bool actualResult = env.TryLocateExecutable(execName, out string actualPath);
66+
67+
Assert.True(actualResult);
68+
Assert.Equal(expectedPath, actualPath);
69+
}
70+
}
71+
}

‎src/shared/Microsoft.Git.CredentialManager/CommandContext.cs

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT license.
33
using System;
4+
using System.IO;
45
using Microsoft.Git.CredentialManager.Interop.Linux;
56
using Microsoft.Git.CredentialManager.Interop.MacOS;
67
using Microsoft.Git.CredentialManager.Interop.Posix;
@@ -86,9 +87,10 @@ public CommandContext()
8687
SystemPrompts = new WindowsSystemPrompts();
8788
Environment = new WindowsEnvironment(FileSystem);
8889
Terminal = new WindowsTerminal(Trace);
90+
string gitPath = GetGitPath(Environment, FileSystem);
8991
Git = new GitProcess(
9092
Trace,
91-
Environment.LocateExecutable("git.exe"),
93+
gitPath,
9294
FileSystem.GetCurrentDirectory()
9395
);
9496
Settings = new Settings(Environment, Git);
@@ -101,9 +103,10 @@ public CommandContext()
101103
SystemPrompts = new MacOSSystemPrompts();
102104
Environment = new PosixEnvironment(FileSystem);
103105
Terminal = new PosixTerminal(Trace);
106+
string gitPath = GetGitPath(Environment, FileSystem);
104107
Git = new GitProcess(
105108
Trace,
106-
Environment.LocateExecutable("git"),
109+
gitPath,
107110
FileSystem.GetCurrentDirectory()
108111
);
109112
Settings = new Settings(Environment, Git);
@@ -117,9 +120,10 @@ public CommandContext()
117120
SystemPrompts = new LinuxSystemPrompts();
118121
Environment = new PosixEnvironment(FileSystem);
119122
Terminal = new PosixTerminal(Trace);
123+
string gitPath = GetGitPath(Environment, FileSystem);
120124
Git = new GitProcess(
121125
Trace,
122-
Environment.LocateExecutable("git"),
126+
gitPath,
123127
FileSystem.GetCurrentDirectory()
124128
);
125129
Settings = new Settings(Environment, Git);
@@ -140,6 +144,25 @@ public CommandContext()
140144
SystemPrompts.ParentWindowId = Settings.ParentWindowId;
141145
}
142146

147+
private static string GetGitPath(IEnvironment environment, IFileSystem fileSystem)
148+
{
149+
string programName = PlatformUtils.IsWindows() ? "git.exe" : "git";
150+
151+
// Use the GIT_EXEC_PATH environment variable if set
152+
if (environment.Variables.TryGetValue(Constants.EnvironmentVariables.GitExecutablePath,
153+
out string gitExecPath))
154+
{
155+
string candidatePath = Path.Combine(gitExecPath, programName);
156+
if (fileSystem.FileExists(candidatePath))
157+
{
158+
return candidatePath;
159+
}
160+
}
161+
162+
// Otherwise try to locate the git(.exe) on the current PATH
163+
return environment.LocateExecutable(programName);
164+
}
165+
143166
#region ICommandContext
144167

145168
public ISettings Settings { get; }

‎src/shared/Microsoft.Git.CredentialManager/Constants.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public static class EnvironmentVariables
5656
public const string GcmCredNamespace = "GCM_NAMESPACE";
5757
public const string GcmCredentialStore = "GCM_CREDENTIAL_STORE";
5858
public const string GcmPlaintextStorePath = "GCM_PLAINTEXT_STORE_PATH";
59+
public const string GitExecutablePath = "GIT_EXEC_PATH";
5960
}
6061

6162
public static class Http

‎src/shared/Microsoft.Git.CredentialManager/EnvironmentBase.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT license.
33
using System;
44
using System.Collections.Generic;
5+
using System.IO;
56
using System.Linq;
67

78
namespace Microsoft.Git.CredentialManager

‎src/shared/Microsoft.Git.CredentialManager/InternalsVisibleTo.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
using System.Runtime.CompilerServices;
2+
3+
[assembly: InternalsVisibleTo("Microsoft.Git.CredentialManager.Tests")]
4+

‎src/shared/Microsoft.Git.CredentialManager/Interop/Posix/PosixEnvironment.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ protected override string[] SplitPathVariable(string value)
3333

3434
public override bool TryLocateExecutable(string program, out string path)
3535
{
36+
// The "which" utility scans over the PATH and does not include the current working directory
37+
// (unlike the equivalent "where.exe" on Windows), which is exactly what we want. Let's use it.
3638
const string whichPath = "/usr/bin/which";
3739
var psi = new ProcessStartInfo(whichPath, program)
3840
{

‎src/shared/Microsoft.Git.CredentialManager/Interop/Windows/WindowsEnvironment.cs

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,14 @@ namespace Microsoft.Git.CredentialManager.Interop.Windows
1111
{
1212
public class WindowsEnvironment : EnvironmentBase
1313
{
14-
public WindowsEnvironment(IFileSystem fileSystem) : base(fileSystem)
14+
public WindowsEnvironment(IFileSystem fileSystem)
15+
: this(fileSystem, GetCurrentVariables()) { }
16+
17+
internal WindowsEnvironment(IFileSystem fileSystem, IReadOnlyDictionary<string, string> variables)
18+
: base(fileSystem)
1519
{
16-
Variables = GetCurrentVariables();
20+
EnsureArgument.NotNull(variables, nameof(variables));
21+
Variables = variables;
1722
}
1823

1924
#region EnvironmentBase
@@ -67,34 +72,24 @@ public override void RemoveDirectoryFromPath(string directoryPath, EnvironmentVa
6772

6873
public override bool TryLocateExecutable(string program, out string path)
6974
{
70-
string wherePath = Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.System), "where.exe");
71-
var psi = new ProcessStartInfo(wherePath, program)
72-
{
73-
UseShellExecute = false,
74-
RedirectStandardOutput = true
75-
};
76-
77-
using (var where = new Process {StartInfo = psi})
75+
// Don't use "where.exe" on Windows as this includes the current working directory
76+
// and we don't want to enumerate this location; only the PATH.
77+
if (Variables.TryGetValue("PATH", out string pathValue))
7878
{
79-
where.Start();
80-
where.WaitForExit();
81-
82-
switch (where.ExitCode)
79+
string[] paths = SplitPathVariable(pathValue);
80+
foreach (var basePath in paths)
8381
{
84-
case 0: // found
85-
string stdout = where.StandardOutput.ReadToEnd();
86-
string[] results = stdout.Split(new[] {'\r', '\n'}, StringSplitOptions.RemoveEmptyEntries);
87-
path = results.First();
82+
string candidatePath = Path.Combine(basePath, program);
83+
if (FileSystem.FileExists(candidatePath))
84+
{
85+
path = candidatePath;
8886
return true;
89-
90-
case 1: // not found
91-
path = null;
92-
return false;
93-
94-
default:
95-
throw new Exception($"Unknown error locating '{program}' using where.exe. Exit code: {where.ExitCode}.");
87+
}
9688
}
9789
}
90+
91+
path = null;
92+
return false;
9893
}
9994

10095
#endregion

‎src/shared/TestInfrastructure/Objects/TestCommandContext.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public TestCommandContext()
2020
CredentialStore = new TestCredentialStore();
2121
HttpClientFactory = new TestHttpClientFactory();
2222
Git = new TestGit();
23-
Environment = new TestEnvironment();
23+
Environment = new TestEnvironment(FileSystem);
2424
SystemPrompts = new TestSystemPrompts();
2525

2626
Settings = new TestSettings {Environment = Environment, GitConfiguration = Git.GlobalConfiguration};

‎src/shared/TestInfrastructure/Objects/TestEnvironment.cs

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,15 @@ namespace Microsoft.Git.CredentialManager.Tests.Objects
99
{
1010
public class TestEnvironment : IEnvironment
1111
{
12+
private readonly IFileSystem _fileSystem;
1213
private readonly IEqualityComparer<string> _pathComparer;
1314
private readonly IEqualityComparer<string> _envarComparer;
1415
private readonly string _envPathSeparator;
1516

16-
public TestEnvironment(string envPathSeparator = null, IEqualityComparer<string> pathComparer = null, IEqualityComparer<string> envarComparer = null)
17+
public TestEnvironment(IFileSystem fileSystem = null, string envPathSeparator = null, IEqualityComparer<string> pathComparer = null, IEqualityComparer<string> envarComparer = null)
1718
{
19+
_fileSystem = fileSystem ?? new TestFileSystem();
20+
1821
// Use the current platform separators and comparison types by default
1922
_envPathSeparator = envPathSeparator ?? (PlatformUtils.IsWindows() ? ";" : ":");
2023

@@ -28,16 +31,12 @@ public TestEnvironment(string envPathSeparator = null, IEqualityComparer<string>
2831
? StringComparer.Ordinal
2932
: StringComparer.OrdinalIgnoreCase);
3033

31-
_envPathSeparator = envPathSeparator;
3234
Variables = new Dictionary<string, string>(_envarComparer);
33-
WhichFiles = new Dictionary<string, ICollection<string>>(_pathComparer);
3435
Symlinks = new Dictionary<string, string>(_pathComparer);
3536
}
3637

3738
public IDictionary<string, string> Variables { get; set; }
3839

39-
public IDictionary<string, ICollection<string>> WhichFiles { get; set; }
40-
4140
public IDictionary<string, string> Symlinks { get; set; }
4241

4342
public IList<string> Path
@@ -82,18 +81,18 @@ public void RemoveDirectoryFromPath(string directoryPath, EnvironmentVariableTar
8281

8382
public bool TryLocateExecutable(string program, out string path)
8483
{
85-
if (WhichFiles.TryGetValue(program, out ICollection<string> paths))
84+
if (Variables.TryGetValue("PATH", out string pathValue))
8685
{
87-
path = paths.First();
88-
return true;
89-
}
90-
91-
if (!System.IO.Path.HasExtension(program) && PlatformUtils.IsWindows())
92-
{
93-
// If we're testing on a Windows platform, don't have a file extension, and were unable to locate
94-
// the executable file.. try appending .exe.
95-
path = WhichFiles.TryGetValue($"{program}.exe", out paths) ? paths.First() : null;
96-
return !(path is null);
86+
string[] paths = pathValue.Split(new[]{_envPathSeparator}, StringSplitOptions.None);
87+
foreach (var basePath in paths)
88+
{
89+
string candidatePath = System.IO.Path.Combine(basePath, program);
90+
if (_fileSystem.FileExists(candidatePath))
91+
{
92+
path = candidatePath;
93+
return true;
94+
}
95+
}
9796
}
9897

9998
path = null;

0 commit comments

Comments
 (0)