diff --git a/dotnet/targets/Xamarin.Shared.Sdk.targets b/dotnet/targets/Xamarin.Shared.Sdk.targets
index a40c8eeb6a4..edf3226b5ca 100644
--- a/dotnet/targets/Xamarin.Shared.Sdk.targets
+++ b/dotnet/targets/Xamarin.Shared.Sdk.targets
@@ -2098,6 +2098,7 @@
Condition="'$(IsMacEnabled)' == 'true'"
SessionId="$(BuildSessionId)"
DynamicLibrary="@(_DynamicLibraryToReidentify)"
+ IntermediateNativeLibraryDir="$(_IntermediateNativeLibraryDir)"
SdkDevPath="$(_SdkDevPath)"
/>
diff --git a/msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx b/msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx
index 0741be004f3..0602550cda8 100644
--- a/msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx
+++ b/msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx
@@ -1666,4 +1666,28 @@
Shown when an MSBuild task writes to the console instead of using MSBuild logging.
{0} - The stack trace of the offending call.
+
+ Ignoring the metadata '{0}' for the native reference '{1}' in the binding resource package '{2}': this metadata is computed by the build and can't be set in a binding resource package manifest.
+ Shown when a binding resource package's manifest contains build-controlled path/layout/identity metadata that isn't allowed to be set from a manifest.
+{0} - The name of the metadata (e.g. "RelativePath").
+{1} - The name of the native reference (e.g. "libfoo.dylib").
+{2} - The path to the binding resource package.
+
+
+ The native reference '{0}' in the binding resource package '{1}' is invalid: the name must be a relative path without '..' segments.
+ Shown when a binding resource package's manifest contains a native reference whose name is an absolute path or contains '..' traversal segments.
+{0} - The invalid native reference name.
+{1} - The path to the binding resource package.
+
+
+ The native library can't be reidentified to '{0}' because that path is outside the intended output directory '{1}'.
+ Shown when the computed reidentified path for a native library would write outside the intended intermediate output directory.
+{0} - The computed (out-of-bounds) reidentified path.
+{1} - The intended output directory.
+
+
+ Ignoring a native reference with no name in the binding resource package '{0}'.
+ Shown when a binding resource package's manifest contains a native reference whose 'Name' attribute is missing or empty.
+{0} - The path to the binding resource package.
+
diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/InstallNameTool.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/InstallNameTool.cs
index 341c6dbe1b0..9e189dfab35 100644
--- a/msbuild/Xamarin.MacDev.Tasks/Tasks/InstallNameTool.cs
+++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/InstallNameTool.cs
@@ -7,6 +7,7 @@
using Microsoft.Build.Framework;
using Xamarin.Messaging.Build.Client;
+using Xamarin.Utils;
#nullable enable
@@ -15,6 +16,12 @@ public class InstallNameTool : XamarinTask, ITaskCallback {
[Required]
public ITaskItem [] DynamicLibrary { get; set; } = [];
+ // The intended output directory for reidentified native libraries. Used to make sure we never
+ // write outside this directory, even if the reidentified path was influenced by metadata that
+ // originates from a (passive) binding resource package manifest.
+ [Required]
+ public string IntermediateNativeLibraryDir { get; set; } = "";
+
// This isn't consumed from the targets files, but it's needed for VSX to create corresponding
// files on Windows.
[Output]
@@ -34,6 +41,17 @@ public override bool Execute ()
// Make sure we use the correct path separator, these are relative paths, so it doesn't look
// like MSBuild does the conversion automatically.
var target = input.GetMetadata ("ReidentifiedPath").Replace ('\\', Path.DirectorySeparatorChar);
+
+ // Defense-in-depth: the 'ReidentifiedPath' can be influenced by metadata that originates
+ // from a (passive) binding resource package manifest. Make sure we
+ // never create directories or write files outside the intended intermediate output
+ // directory, even if the path contains traversal segments, is absolute, or uses symlinks.
+ if (!PathUtils.IsPathContained (IntermediateNativeLibraryDir, target)) {
+ Log.LogError (MSBStrings.E7181 /* The native library can't be reidentified to '{0}' because that path is outside the intended output directory '{1}'. */, target, IntermediateNativeLibraryDir);
+ processes [i] = System.Threading.Tasks.Task.CompletedTask;
+ continue;
+ }
+
var temporaryTarget = target + ".tmp";
// install_name_tool modifies the file in-place, so copy it first to a temporary file first.
@@ -48,11 +66,17 @@ public override bool Execute ()
arguments.Add (temporaryTarget);
processes [i] = ExecuteAsync ("xcrun", arguments).ContinueWith ((v) => {
- if (v.IsFaulted)
+ if (v.IsFaulted) {
+ // install_name_tool faulted; don't leave the temporary copy behind.
+ File.Delete (temporaryTarget);
throw v.Exception;
- if (v.Status == TaskStatus.RanToCompletion) {
+ }
+ if (v.Status == TaskStatus.RanToCompletion && v.Result.ExitCode == 0) {
File.Delete (target);
File.Move (temporaryTarget, target);
+ } else {
+ // install_name_tool failed; don't leave the temporary copy behind.
+ File.Delete (temporaryTarget);
}
});
@@ -61,6 +85,9 @@ public override bool Execute ()
Task.WaitAll (processes);
+ // Drop any items we skipped because their reidentified path wasn't contained.
+ ReidentifiedDynamicLibrary = ReidentifiedDynamicLibrary.Where (item => item is not null).ToArray ();
+
return !Log.HasLoggedErrors;
}
diff --git a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs
index 73c8dcc7dee..da54198cb28 100644
--- a/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs
+++ b/msbuild/Xamarin.MacDev.Tasks/Tasks/ResolveNativeReferences.cs
@@ -299,6 +299,42 @@ void SetMetadataNativeLibrary (ITaskItem item, string nativeLibraryPath)
item.SetMetadata ("RelativePath", Path.Combine (FrameworksDirectory, Path.GetFileName (Path.GetDirectoryName (item.ItemSpec)!)));
}
+ // Metadata we must NOT copy from a binding resource package's 'manifest' file. A binding resource
+ // package is passive data that may come from a restored package, so its manifest must only
+ // describe native-reference content - it must not be able to inject the build-controlled
+ // path/layout/identity metadata below, which is used to compute where a native library is laid
+ // out, reidentified or published (and could otherwise redirect output outside the intended output
+ // directory). Everything else - including binding-defined metadata such as 'NoDSymUtil' or
+ // 'NoSymbolStrip' - is legitimate native-reference content and is copied as-is.
+ static readonly HashSet blockedManifestMetadata = new HashSet (StringComparer.OrdinalIgnoreCase) {
+ "RelativePath",
+ "ReidentifiedPath",
+ "ComputedRelativePath",
+ "DynamicLibraryId",
+ "PublishFolderType",
+ "TargetDirectory",
+ "SourceDirectory",
+ };
+
+ // Returns true if 'path' is not a safe relative subpath: i.e. it's rooted/absolute, drive-qualified,
+ // or contains a '..' traversal segment. Used to validate untrusted path-shaped values that come
+ // from a binding resource package manifest before they're combined with a directory.
+ static bool IsUnsafeRelativePath (string path)
+ {
+ if (string.IsNullOrEmpty (path))
+ return false;
+ // Drive-qualified (e.g. "C:..." on Windows) or alternate-data-stream separators.
+ if (path.IndexOf (':') >= 0)
+ return true;
+ if (Path.IsPathRooted (path))
+ return true;
+ foreach (var segment in path.Split ('/', '\\')) {
+ if (segment == "..")
+ return true;
+ }
+ return false;
+ }
+
void ProcessSidecar (ITaskItem r, string resources, List native_frameworks, List createdFiles, CancellationToken? cancellationToken)
{
if (!TryGetSidecarManifest (Log, resources, out var manifestContents))
@@ -310,6 +346,19 @@ void ProcessSidecar (ITaskItem r, string resources, List native_frame
foreach (XmlNode referenceNode in document.GetElementsByTagName ("NativeReference")) {
ITaskItem t = new TaskItem (r);
var name = referenceNode.Attributes? ["Name"]?.Value.Trim ('\\', '/') ?? string.Empty;
+ // A native reference without a (usable) name can't be processed; skip it instead of falling
+ // through and accidentally treating the managed reference itself as a native reference.
+ if (string.IsNullOrEmpty (name)) {
+ Log.LogWarning (MSBStrings.W7182 /* Ignoring a native reference with no name in the binding resource package '{0}'. */, r.ItemSpec);
+ continue;
+ }
+ // The 'Name' is combined with the binding resource package's directory to locate (and
+ // potentially extract) the native reference, so make sure it can't be used to escape the
+ // binding resource package using an absolute path or '..' traversal segments.
+ if (IsUnsafeRelativePath (name)) {
+ Log.LogError (MSBStrings.E7180 /* The native reference '{0}' in the binding resource package '{1}' is invalid: the name must be a relative path without '..' segments. */, name, r.ItemSpec);
+ continue;
+ }
if (name.EndsWith (".xcframework", StringComparison.Ordinal) || name.EndsWith (".xcframework.zip", StringComparison.Ordinal)) {
if (!TryResolveXCFramework (this, TargetFrameworkMoniker, SdkIsSimulator, Architectures, resources, name, GetIntermediateDecompressionDir (resources), createdFiles, cancellationToken, out var nativeLibraryPath))
continue;
@@ -359,9 +408,24 @@ void ProcessSidecar (ITaskItem r, string resources, List native_frame
t.SetMetadata ("LinkWithSwiftSystemLibraries", "False");
t.SetMetadata ("SmartLink", "True");
- // values from manifest, overriding defaults if provided
- foreach (XmlNode attribute in referenceNode.ChildNodes)
- t.SetMetadata (attribute.Name, attribute.InnerText);
+ // Values from the manifest, overriding the defaults above if provided. Skip the
+ // build-controlled path/layout/identity metadata: the manifest is passive data and must not
+ // be able to inject metadata that could redirect output outside the intended directory.
+ foreach (XmlNode attribute in referenceNode.ChildNodes) {
+ if (attribute.NodeType != XmlNodeType.Element)
+ continue;
+ if (blockedManifestMetadata.Contains (attribute.Name)) {
+ Log.LogWarning (MSBStrings.W7179 /* Ignoring the metadata '{0}' for the native reference '{1}' in the binding resource package '{2}': this metadata is computed by the build and can't be set in a binding resource package manifest. */, attribute.Name, name, r.ItemSpec);
+ continue;
+ }
+ try {
+ t.SetMetadata (attribute.Name, attribute.InnerText);
+ } catch (ArgumentException) {
+ // Reserved MSBuild metadata names (e.g. 'FullPath') can't be set and shouldn't appear
+ // in a manifest; ignore (and warn about) them rather than failing the build.
+ Log.LogWarning (MSBStrings.W7179 /* Ignoring the metadata '{0}' for the native reference '{1}' in the binding resource package '{2}': this metadata is computed by the build and can't be set in a binding resource package manifest. */, attribute.Name, name, r.ItemSpec);
+ }
+ }
native_frameworks.Add (t);
}
diff --git a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/InstallNameToolTaskTest.cs b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/InstallNameToolTaskTest.cs
new file mode 100644
index 00000000000..c56e02f0186
--- /dev/null
+++ b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/InstallNameToolTaskTest.cs
@@ -0,0 +1,85 @@
+using System.IO;
+
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+
+using NUnit.Framework;
+
+#nullable enable
+
+namespace Xamarin.MacDev.Tasks {
+
+ // Tests for the InstallNameTool task: a reidentified native library must never be written outside the
+ // intended intermediate output directory, even if the 'ReidentifiedPath' was influenced by metadata
+ // that originates from a (passive) binding resource package manifest.
+ [TestFixture]
+ public class InstallNameToolTaskTest : TestBase {
+
+ [TestCase ("traversal")]
+ [TestCase ("absolute")]
+ [TestCase ("mixedseparators")]
+ public void RefusesToWriteOutOfRoot (string kind)
+ {
+ var tmp = Cache.CreateTemporaryDirectory ();
+ var root = Path.Combine (tmp, "obj", "nativelibraries");
+ Directory.CreateDirectory (root);
+ var src = Path.Combine (tmp, "libpayload.dylib");
+ File.WriteAllText (src, "fake dylib");
+ var escapeTarget = Path.Combine (tmp, "ESCAPED", "libpayload.dylib");
+
+ string reidentifiedPath;
+ switch (kind) {
+ case "traversal":
+ reidentifiedPath = Path.Combine (root, "..", "..", "ESCAPED", "libpayload.dylib");
+ break;
+ case "absolute":
+ reidentifiedPath = escapeTarget;
+ break;
+ case "mixedseparators":
+ reidentifiedPath = root + @"\..\..\ESCAPED\libpayload.dylib";
+ break;
+ default:
+ throw new System.NotImplementedException (kind);
+ }
+
+ var task = CreateTask ();
+ task.IntermediateNativeLibraryDir = root;
+ var item = new TaskItem (src);
+ item.SetMetadata ("ReidentifiedPath", reidentifiedPath);
+ item.SetMetadata ("DynamicLibraryId", "@executable_path/libpayload.dylib");
+ task.DynamicLibrary = new ITaskItem [] { item };
+
+ ExecuteTask (task, 1);
+
+ // Nothing was created outside the intended directory (not even the temporary file).
+ Assert.That (Path.Combine (tmp, "ESCAPED"), Does.Not.Exist, "no escaped directory");
+ Assert.That (escapeTarget, Does.Not.Exist, "no escaped file");
+ Assert.That (escapeTarget + ".tmp", Does.Not.Exist, "no escaped temp file");
+ }
+
+ [Test]
+ public void CleansUpTemporaryFileWhenInstallNameToolFails ()
+ {
+ // When install_name_tool fails (here: the input isn't a valid Mach-O file) the temporary copy
+ // must not be left behind.
+ var tmp = Cache.CreateTemporaryDirectory ();
+ var root = Path.Combine (tmp, "obj", "nativelibraries");
+ Directory.CreateDirectory (root);
+ var src = Path.Combine (tmp, "libpayload.dylib");
+ File.WriteAllText (src, "this is not a valid Mach-O file");
+ var target = Path.Combine (root, "Contents", "MonoBundle", "libpayload.dylib");
+
+ var task = CreateTask ();
+ task.IntermediateNativeLibraryDir = root;
+ var item = new TaskItem (src);
+ item.SetMetadata ("ReidentifiedPath", target);
+ item.SetMetadata ("DynamicLibraryId", "@executable_path/libpayload.dylib");
+ task.DynamicLibrary = new ITaskItem [] { item };
+
+ ExecuteTask (task, 1);
+
+ Assert.That (target + ".tmp", Does.Not.Exist, "temporary file cleaned up");
+ Assert.That (target, Does.Not.Exist, "target not created on failure");
+ }
+ }
+}
diff --git a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ResolveNativeReferencesSidecarTest.cs b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ResolveNativeReferencesSidecarTest.cs
new file mode 100644
index 00000000000..aa1df655b54
--- /dev/null
+++ b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/ResolveNativeReferencesSidecarTest.cs
@@ -0,0 +1,139 @@
+using System.IO;
+using System.Linq;
+
+using Microsoft.Build.Framework;
+using Microsoft.Build.Utilities;
+
+using NUnit.Framework;
+
+#nullable enable
+
+namespace Xamarin.MacDev.Tasks {
+
+ // Regression tests for the binding resource package 'manifest' handling: a manifest is passive data
+ // that may come from a restored package, so it must not be able to inject path/layout/identity
+ // metadata that could redirect this task's (or a downstream task's) output outside the intended
+ // output directory.
+ [TestFixture]
+ public class ResolveNativeReferencesSidecarTest : TestBase {
+
+ ResolveNativeReferences CreateSidecarTask (string manifestContents, out string tmpdir)
+ {
+ tmpdir = Cache.CreateTemporaryDirectory ();
+ var resources = Path.Combine (tmpdir, "Binding.resources");
+ Directory.CreateDirectory (resources);
+ File.WriteAllText (Path.Combine (resources, "manifest"), manifestContents);
+ // Create the (fake) native library referenced by the manifest, for realism.
+ File.WriteAllText (Path.Combine (resources, "libpayload.dylib"), "fake dylib");
+
+ var task = CreateTask ();
+ task.Architectures = "arm64";
+ task.FrameworksDirectory = "Frameworks";
+ task.IntermediateOutputPath = Path.Combine (tmpdir, "obj");
+ task.SdkIsSimulator = false;
+ task.TargetFrameworkMoniker = "net9.0-macos";
+ task.References = new ITaskItem [] { new TaskItem (Path.Combine (tmpdir, "Binding.dll")) };
+ return task;
+ }
+
+ [Test]
+ public void StripsUnsafeManifestMetadata ()
+ {
+ var manifest = @"
+
+ Dynamic
+ True
+ CoreFoundation
+ true
+ true
+ ../../../../../../tmp/escape/libpayload.dylib
+ /tmp/escape/libpayload.dylib
+ ../../escape/libpayload.dylib
+ @executable_path/../../../../escape
+ Unknown
+ /tmp/escape
+ /tmp/escape
+
+";
+ var task = CreateSidecarTask (manifest, out var _);
+
+ ExecuteTask (task, 0);
+
+ var item = task.NativeFrameworks!.Single (v => v.GetMetadata ("Kind") == "Dynamic");
+
+ // Legitimate native-reference metadata (standard + binding-defined) is preserved.
+ Assert.That (item.GetMetadata ("ForceLoad"), Is.EqualTo ("True"), "ForceLoad");
+ Assert.That (item.GetMetadata ("Frameworks"), Is.EqualTo ("CoreFoundation"), "Frameworks");
+ Assert.That (item.GetMetadata ("NoDSymUtil"), Is.EqualTo ("true"), "NoDSymUtil");
+ Assert.That (item.GetMetadata ("NoSymbolStrip"), Is.EqualTo ("true"), "NoSymbolStrip");
+
+ // Build-controlled path/layout/identity metadata is NOT copied from the manifest.
+ foreach (var blocked in new [] { "RelativePath", "ReidentifiedPath", "ComputedRelativePath", "DynamicLibraryId", "TargetDirectory", "SourceDirectory" })
+ Assert.That (item.GetMetadata (blocked), Is.Empty, blocked);
+ // The code-set PublishFolderType survives (the manifest's value is ignored).
+ Assert.That (item.GetMetadata ("PublishFolderType"), Is.EqualTo ("DynamicLibrary"), "PublishFolderType");
+
+ // A warning is emitted for each ignored (blocked) metadata - and only for those (use the quoted
+ // name so e.g. 'ComputedRelativePath' isn't counted as 'RelativePath').
+ var warnings = Engine.Logger.WarningsEvents.Select (v => v.Message ?? "").ToArray ();
+ foreach (var blocked in new [] { "RelativePath", "ReidentifiedPath", "ComputedRelativePath", "DynamicLibraryId", "PublishFolderType", "TargetDirectory", "SourceDirectory" })
+ Assert.That (warnings.Count (v => v.Contains ($"'{blocked}'")), Is.EqualTo (1), $"{blocked} warning");
+ foreach (var allowed in new [] { "NoDSymUtil", "NoSymbolStrip", "ForceLoad", "Frameworks" })
+ Assert.That (warnings.Count (v => v.Contains ($"'{allowed}'")), Is.EqualTo (0), $"no warning for {allowed}");
+ }
+
+ [Test]
+ public void IgnoresReservedMetadata ()
+ {
+ // A crafted manifest must not be able to crash the task with reserved MSBuild metadata names
+ // (TaskItem.SetMetadata throws an ArgumentException for those).
+ var manifest = @"
+
+ Dynamic
+ evil
+ evil
+
+";
+ var task = CreateSidecarTask (manifest, out var _);
+
+ // The task completes without throwing/erroring (reaching this assertion means it didn't crash).
+ ExecuteTask (task, 0);
+
+ var item = task.NativeFrameworks!.Single (v => v.GetMetadata ("Kind") == "Dynamic");
+ // The reserved metadata keeps its item-spec-derived value; the manifest can't override it.
+ Assert.That (item.GetMetadata ("Filename"), Is.EqualTo ("libpayload"), "Filename");
+ }
+
+ [Test]
+ public void IgnoresNativeReferenceWithoutName ()
+ {
+ var manifest = @"
+
+ Dynamic
+
+";
+ var task = CreateSidecarTask (manifest, out var _);
+
+ ExecuteTask (task, 0);
+
+ // The nameless native reference is skipped (the managed reference itself isn't added).
+ Assert.That (task.NativeFrameworks!, Is.Empty, "no native frameworks");
+ Assert.That (Engine.Logger.WarningsEvents.Count (v => (v.Message ?? "").Contains ("no name")), Is.EqualTo (1), "no-name warning");
+ }
+
+ [Test]
+ public void RejectsTraversalInName ()
+ {
+ var manifest = @"
+
+ Dynamic
+
+";
+ var task = CreateSidecarTask (manifest, out var _);
+
+ ExecuteTask (task, 1);
+
+ Assert.That (Engine.Logger.ErrorEvents.Single ().Message, Does.Contain ("'..'"), "error mentions traversal");
+ }
+ }
+}
diff --git a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/UtilityTests.cs b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/UtilityTests.cs
index 4dfc91c8446..10ac1a27dc0 100644
--- a/tests/msbuild/Xamarin.MacDev.Tasks.Tests/UtilityTests.cs
+++ b/tests/msbuild/Xamarin.MacDev.Tasks.Tests/UtilityTests.cs
@@ -21,5 +21,60 @@ public void TestAbsoluteToRelativePath ()
rpath = PathUtils.AbsoluteToRelative ("/Users/user/source/Project", "/Users/user/Source/Project/Info.plist");
Assert.That (rpath, Is.EqualTo ("Info.plist"), "#1");
}
+
+ [Test]
+ public void IsPathContained_Contained ()
+ {
+ var tmp = Cache.CreateTemporaryDirectory ();
+ var root = Path.Combine (tmp, "obj", "nativelibraries");
+ Directory.CreateDirectory (root);
+
+ Assert.That (PathUtils.IsPathContained (root, Path.Combine (root, "Contents", "MonoBundle", "lib.dylib")), Is.True, "subdir");
+ Assert.That (PathUtils.IsPathContained (root, Path.Combine (root, "lib.dylib")), Is.True, "direct child");
+ // A trailing separator on the root must not change the result.
+ Assert.That (PathUtils.IsPathContained (root + Path.DirectorySeparatorChar, Path.Combine (root, "lib.dylib")), Is.True, "trailing separator root");
+ // A Windows-style (backslash) root must be normalized to match a slash-normalized target
+ // (this happens on remote Windows -> Mac builds).
+ Assert.That (PathUtils.IsPathContained (root.Replace (Path.DirectorySeparatorChar, '\\'), Path.Combine (root, "lib.dylib")), Is.True, "backslash root");
+ }
+
+ [Test]
+ public void IsPathContained_NotContained ()
+ {
+ var tmp = Cache.CreateTemporaryDirectory ();
+ var root = Path.Combine (tmp, "obj", "nativelibraries");
+ Directory.CreateDirectory (root);
+
+ // '..' traversal that escapes the root.
+ Assert.That (PathUtils.IsPathContained (root, Path.Combine (root, "..", "..", "escape.dylib")), Is.False, "traversal");
+ // An absolute path outside the root.
+ Assert.That (PathUtils.IsPathContained (root, Path.Combine (tmp, "escape.dylib")), Is.False, "outside");
+ // A sibling directory that merely shares the root as a string prefix must not be considered contained.
+ Assert.That (PathUtils.IsPathContained (root, root + "EVIL" + Path.DirectorySeparatorChar + "lib.dylib"), Is.False, "sibling prefix");
+ // The root itself isn't a contained target (it's a directory, not a file under the root).
+ Assert.That (PathUtils.IsPathContained (root, root), Is.False, "root itself");
+ // The root itself with trailing separator(s) is still not a contained target.
+ Assert.That (PathUtils.IsPathContained (root, root + Path.DirectorySeparatorChar), Is.False, "root itself + separator");
+ Assert.That (PathUtils.IsPathContained (root + Path.DirectorySeparatorChar, root + Path.DirectorySeparatorChar), Is.False, "root+sep vs root+sep");
+ // Empty inputs are never contained.
+ Assert.That (PathUtils.IsPathContained ("", Path.Combine (root, "lib.dylib")), Is.False, "empty root");
+ Assert.That (PathUtils.IsPathContained (root, ""), Is.False, "empty target");
+ }
+
+ [Test]
+ public void IsPathContained_SymlinkEscape ()
+ {
+ var tmp = Cache.CreateTemporaryDirectory ();
+ var root = Path.Combine (tmp, "obj", "nativelibraries");
+ Directory.CreateDirectory (root);
+ var outside = Path.Combine (tmp, "outside");
+ Directory.CreateDirectory (outside);
+
+ // A symlink inside the root that points outside the root must not be usable to escape.
+ var link = Path.Combine (root, "link");
+ PathUtils.CreateSymlink (link, outside);
+
+ Assert.That (PathUtils.IsPathContained (root, Path.Combine (link, "evil.dylib")), Is.False, "symlink escape");
+ }
}
}
diff --git a/tools/common/PathUtils.cs b/tools/common/PathUtils.cs
index 355253f5c2a..dca93cb74b2 100644
--- a/tools/common/PathUtils.cs
+++ b/tools/common/PathUtils.cs
@@ -72,6 +72,56 @@ static char ToOrdinalIgnoreCase (char c)
}
}
+ ///
+ /// Returns true if is located strictly within
+ /// after fully canonicalizing both paths (making them absolute, resolving '..'/'.' segments and
+ /// resolving symbolic links). Either path may point to a location that doesn't exist yet. This is
+ /// useful to make sure a computed output path can't escape an intended output directory, even if
+ /// the path was influenced by untrusted input (e.g. via traversal segments, absolute paths or
+ /// symlinks).
+ ///
+ public static bool IsPathContained (string root, string target)
+ {
+ if (string.IsNullOrEmpty (root) || string.IsNullOrEmpty (target))
+ return false;
+
+ var canonicalRoot = CanonicalizeForContainment (root).TrimEnd (Path.DirectorySeparatorChar);
+ var canonicalTarget = CanonicalizeForContainment (target).TrimEnd (Path.DirectorySeparatorChar);
+
+ // The target must be strictly inside the root, not the root directory itself.
+ if (string.Equals (canonicalTarget, canonicalRoot, StringComparison.Ordinal))
+ return false;
+
+ return canonicalTarget.StartsWith (canonicalRoot + Path.DirectorySeparatorChar, StringComparison.Ordinal);
+ }
+
+ // Canonicalizes a path for a containment check: makes it absolute and resolves '..'/'.' segments,
+ // then resolves symbolic links for the longest existing prefix (the target usually doesn't exist
+ // yet, so we resolve symlinks on the nearest existing ancestor and re-append the remainder).
+ static string CanonicalizeForContainment (string path)
+ {
+ // The path can come from MSBuild with Windows separators (e.g. 'obj\...\nativelibraries\'),
+ // so normalize the separators first.
+ path = path.Replace ('\\', Path.DirectorySeparatorChar);
+ var full = Path.GetFullPath (path);
+
+ var existing = full;
+ var remainder = new List ();
+ while (existing.Length > 0 && !Directory.Exists (existing) && !File.Exists (existing)) {
+ var parent = Path.GetDirectoryName (existing);
+ if (string.IsNullOrEmpty (parent) || parent == existing)
+ break;
+ remainder.Insert (0, Path.GetFileName (existing));
+ existing = parent;
+ }
+
+ var canonical = ResolveSymbolicLinks (existing) ?? existing;
+ foreach (var segment in remainder)
+ canonical = Path.Combine (canonical, segment);
+
+ return canonical;
+ }
+
/// This works like , except that it works as it executing on Windows on all platforms.
/// The directory the return value should be relative to. Must be an absolute path.
/// The path whose relative value should be computed. Must be an absolute path.