Skip to content

Commit efd4e72

Browse files
jonathanpeppersCopilot
jonathanpeppers
and
Copilot
authored
[Java.Interop.Tools.Maven] Assert resolved cache paths stay under CacheDirectory (#1480)
Adds a path-staying-under-CacheDirectory assertion to `CachedMavenRepository`. Exposes a new public API, `GetArtifactFilePath (Artifact, string)`, that returns the on-disk path where an artifact + filename would be cached and throws `InvalidOperationException` if the resolved path would not be under `CacheDirectory`. `TryGetFile`, `TryGetFilePath`, and `GetFilePathAsync` all route through this single method so there is exactly one place that knows the cache layout. The new public API lets callers (such as dotnet/android's `MavenExtensions.DownloadPayload`) stop reconstructing the cache path themselves with their own `Path.Combine` logic and get the assertion for free. Defense-in-depth, hardening. Not security enforcement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 359b764 commit efd4e72

2 files changed

Lines changed: 209 additions & 6 deletions

File tree

‎external/Java.Interop/src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Diagnostics.CodeAnalysis;
23
using System.IO;
34
using System.Threading;
@@ -39,16 +40,15 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr
3940
{
4041
path = null;
4142

42-
var directory = GetArtifactDirectory (artifact);
43-
var file = Path.Combine (directory, filename);
43+
var file = GetArtifactFilePath (artifact, filename);
4444

4545
if (File.Exists (file)) {
4646
path = file;
4747
return true;
4848
}
4949

5050
if (repository.TryGetFile (artifact, filename, out var repo_stream)) {
51-
Directory.CreateDirectory (directory);
51+
Directory.CreateDirectory (GetArtifactDirectory (artifact));
5252

5353
using (var sw = File.Create (file))
5454
using (repo_stream)
@@ -63,14 +63,13 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr
6363

6464
public async Task<string?> GetFilePathAsync (Artifact artifact, string filename, CancellationToken cancellationToken)
6565
{
66-
var directory = GetArtifactDirectory (artifact);
67-
var file = Path.Combine (directory, filename);
66+
var file = GetArtifactFilePath (artifact, filename);
6867

6968
if (File.Exists (file))
7069
return file;
7170

7271
if (repository.TryGetFile (artifact, filename, out var repo_stream)) {
73-
Directory.CreateDirectory (directory);
72+
Directory.CreateDirectory (GetArtifactDirectory (artifact));
7473

7574
using (var sw = File.Create (file))
7675
using (repo_stream)
@@ -83,6 +82,25 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr
8382
return null;
8483
}
8584

85+
/// <summary>
86+
/// Returns the on-disk path where the given <paramref name="artifact"/> + <paramref name="filename"/>
87+
/// would be cached under <see cref="CacheDirectory"/>. Does not download or check for existence.
88+
/// Throws <see cref="InvalidOperationException"/> if the resolved path would not be under
89+
/// <see cref="CacheDirectory"/>.
90+
/// </summary>
91+
public string GetArtifactFilePath (Artifact artifact, string filename)
92+
{
93+
var directory = GetArtifactDirectory (artifact);
94+
var file = Path.Combine (directory, filename);
95+
var full_file = Path.GetFullPath (file);
96+
var full_cache = Path.GetFullPath (CacheDirectory);
97+
if (!full_cache.EndsWith (Path.DirectorySeparatorChar.ToString ()) && !full_cache.EndsWith (Path.AltDirectorySeparatorChar.ToString ()))
98+
full_cache += Path.DirectorySeparatorChar;
99+
if (!full_file.StartsWith (full_cache, StringComparison.Ordinal))
100+
throw new InvalidOperationException ($"Resolved Maven cache path '{full_file}' escapes cache directory '{full_cache}'.");
101+
return full_file;
102+
}
103+
86104
string GetArtifactDirectory (Artifact artifact)
87105
{
88106
var version = artifact.Version;

‎external/Java.Interop/tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
using System;
2+
using System.Diagnostics.CodeAnalysis;
3+
using System.IO;
4+
using System.Threading;
5+
using System.Threading.Tasks;
6+
using Java.Interop.Tools.Maven.Models;
7+
using Java.Interop.Tools.Maven.Repositories;
8+
9+
namespace Java.Interop.Tools.Maven_Tests;
10+
11+
public class CachedMavenRepositoryTests
12+
{
13+
string cache_dir = "";
14+
15+
[SetUp]
16+
public void SetUp ()
17+
{
18+
cache_dir = Path.Combine (Path.GetTempPath (), "Java.Interop.Tools.Maven-Tests", Path.GetRandomFileName ());
19+
Directory.CreateDirectory (cache_dir);
20+
}
21+
22+
[TearDown]
23+
public void TearDown ()
24+
{
25+
if (Directory.Exists (cache_dir))
26+
Directory.Delete (cache_dir, recursive: true);
27+
}
28+
29+
[Test]
30+
public void GetArtifactFilePath_HappyPath_ReturnsExpectedLayout ()
31+
{
32+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
33+
var inner = new StubRepository ("central", artifact, "lib-1.0.0.jar", new byte [] { 1, 2, 3 });
34+
var cache = new CachedMavenRepository (cache_dir, inner);
35+
36+
var expected = Path.GetFullPath (Path.Combine (cache_dir, "central", "com.example", "lib", "1.0.0", "lib-1.0.0.jar"));
37+
var actual = cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar");
38+
39+
Assert.AreEqual (expected, actual);
40+
Assert.IsFalse (File.Exists (actual), "GetArtifactFilePath must not create/download the file.");
41+
}
42+
43+
[Test]
44+
public void TryGetFilePath_HappyPath_DownloadsAndReturnsExpectedPath ()
45+
{
46+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
47+
var content = new byte [] { 1, 2, 3 };
48+
var inner = new StubRepository ("central", artifact, "lib-1.0.0.jar", content);
49+
var cache = new CachedMavenRepository (cache_dir, inner);
50+
51+
var expected = Path.GetFullPath (Path.Combine (cache_dir, "central", "com.example", "lib", "1.0.0", "lib-1.0.0.jar"));
52+
53+
Assert.IsTrue (cache.TryGetFilePath (artifact, "lib-1.0.0.jar", out var path));
54+
Assert.AreEqual (expected, path);
55+
Assert.IsTrue (File.Exists (path));
56+
CollectionAssert.AreEqual (content, File.ReadAllBytes (path!));
57+
}
58+
59+
[Test]
60+
public void GetArtifactFilePath_RelativeFilename_Throws ()
61+
{
62+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
63+
var inner = new ThrowingRepository ("central");
64+
var cache = new CachedMavenRepository (cache_dir, inner);
65+
66+
var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!;
67+
var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar");
68+
var malicious = Path.GetRelativePath (artifact_dir, outside);
69+
70+
Assert.Throws<InvalidOperationException> (() => cache.GetArtifactFilePath (artifact, malicious));
71+
}
72+
73+
[Test]
74+
public void TryGetFilePath_RelativeFilename_Throws ()
75+
{
76+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
77+
var inner = new ThrowingRepository ("central");
78+
var cache = new CachedMavenRepository (cache_dir, inner);
79+
80+
var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!;
81+
var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar");
82+
var malicious = Path.GetRelativePath (artifact_dir, outside);
83+
84+
Assert.Throws<InvalidOperationException> (() => cache.TryGetFilePath (artifact, malicious, out _));
85+
Assert.AreEqual (0, inner.CallCount, "Inner repository must not be consulted for an escaping path.");
86+
}
87+
88+
[Test]
89+
public void TryGetFile_RelativeFilename_Throws ()
90+
{
91+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
92+
var inner = new ThrowingRepository ("central");
93+
var cache = new CachedMavenRepository (cache_dir, inner);
94+
95+
var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!;
96+
var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar");
97+
var malicious = Path.GetRelativePath (artifact_dir, outside);
98+
99+
Assert.Throws<InvalidOperationException> (() => cache.TryGetFile (artifact, malicious, out _));
100+
Assert.AreEqual (0, inner.CallCount, "Inner repository must not be consulted for an escaping path.");
101+
}
102+
103+
[Test]
104+
public void GetFilePathAsync_RelativeFilename_Throws ()
105+
{
106+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
107+
var inner = new ThrowingRepository ("central");
108+
var cache = new CachedMavenRepository (cache_dir, inner);
109+
110+
var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!;
111+
var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar");
112+
var malicious = Path.GetRelativePath (artifact_dir, outside);
113+
114+
Assert.ThrowsAsync<InvalidOperationException> (async () =>
115+
await cache.GetFilePathAsync (artifact, malicious, CancellationToken.None));
116+
Assert.AreEqual (0, inner.CallCount, "Inner repository must not be consulted for an escaping path.");
117+
}
118+
119+
[Test]
120+
public void GetArtifactFilePath_RelativeRepositoryName_Throws ()
121+
{
122+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
123+
var inner = new ThrowingRepository (Path.Combine ("..", Path.GetFileName (cache_dir) + "-sibling"));
124+
var cache = new CachedMavenRepository (cache_dir, inner);
125+
126+
Assert.Throws<InvalidOperationException> (() => cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar"));
127+
}
128+
129+
[Test]
130+
public void GetArtifactFilePath_SiblingPrefixCacheDirectory_Throws ()
131+
{
132+
var artifact = new Artifact ("com.example", "lib", "1.0.0");
133+
var sibling = cache_dir + "-sibling";
134+
var repo_name = Path.GetRelativePath (cache_dir, sibling);
135+
var inner = new ThrowingRepository (repo_name);
136+
var cache = new CachedMavenRepository (cache_dir, inner);
137+
138+
Assert.Throws<InvalidOperationException> (() => cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar"));
139+
}
140+
141+
sealed class StubRepository : IMavenRepository
142+
{
143+
readonly Artifact expected;
144+
readonly string expected_filename;
145+
readonly byte [] content;
146+
147+
public StubRepository (string name, Artifact expected, string filename, byte [] content)
148+
{
149+
Name = name;
150+
this.expected = expected;
151+
this.expected_filename = filename;
152+
this.content = content;
153+
}
154+
155+
public string Name { get; }
156+
157+
public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)] out Stream? stream)
158+
{
159+
if (artifact.GroupId == expected.GroupId && artifact.Id == expected.Id && artifact.Version == expected.Version && filename == expected_filename) {
160+
stream = new MemoryStream (content);
161+
return true;
162+
}
163+
stream = null;
164+
return false;
165+
}
166+
}
167+
168+
sealed class ThrowingRepository : IMavenRepository
169+
{
170+
public ThrowingRepository (string name)
171+
{
172+
Name = name;
173+
}
174+
175+
public string Name { get; }
176+
177+
public int CallCount { get; private set; }
178+
179+
public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)] out Stream? stream)
180+
{
181+
CallCount++;
182+
throw new InvalidOperationException ("Inner repository should not be consulted when the resolved path escapes the cache directory.");
183+
}
184+
}
185+
}

0 commit comments

Comments
 (0)