[msbuild] Validate extracted resource paths in UnpackLibraryResources

msbuild/Xamarin.Localization.MSBuild/MSBStrings.resx

@@ -1689,6 +1689,14 @@
         <value>Ignoring a native reference with no name in the binding resource package '{0}'.</value>
         <comment>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.</comment>
+    </data>
+    <data name="E7183" xml:space="preserve">
+        <value>The resource '{0}' in assembly '{1}' would extract to '{2}' which is outside of the target directory '{3}'.</value>
+        <comment>Shown when an embedded resource would be extracted to a path outside the intended output directory.
+{0} - The resource name.
+{1} - The assembly path.
+{2} - The computed extraction path.
+{3} - The intended output directory.</comment>
     </data>
     <data name="E0192" xml:space="preserve">
         <value>The PrepareAssemblies task failed without reporting a specific error. Please rebuild with increased verbosity for more details.</value>

msbuild/Xamarin.MacDev.Tasks/Tasks/UnpackLibraryResources.cs

@@ -12,6 +12,7 @@
 using Microsoft.Build.Utilities;
 
 using Xamarin.Localization.MSBuild;
+using Xamarin.Utils;
 
 #nullable enable
 
@@ -350,7 +351,12 @@ List<AssemblyContentResource> ExtractContentAssembly (string assembly, string in
 						continue;
 					}
 
-					var path = Path.Combine (intermediatePath, itemType, rpath);
+					var extractionDirectory = Path.Combine (intermediatePath, itemType);
+					var path = Path.Combine (extractionDirectory, rpath);
+					if (!PathUtils.IsPathContained (extractionDirectory, path)) {
+						Log.LogError (MSBStrings.E7183 /* The resource '{0}' in assembly '{1}' would extract to '{2}' which is outside of the target directory '{3}'. */, resourceName, assembly, path, extractionDirectory);
+						continue;
+					}
 					var file = new FileInfo (path);
 
 					var item = new TaskItem (path);

tests/msbuild/Xamarin.MacDev.Tasks.Tests/TaskTests/UnpackLibraryResourcesTests.cs

@@ -0,0 +1,76 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using System.IO;
+
+using Microsoft.Build.Utilities;
+
+using Mono.Cecil;
+
+using NUnit.Framework;
+
+using Xamarin.Tests;
+
+namespace Xamarin.MacDev.Tasks {
+	[TestFixture]
+	public class UnpackLibraryResourcesTests : TestBase {
+
+		static string CreateAssemblyWithResource (string directory, string assemblyName, string resourceName, byte [] content)
+		{
+			var asmPath = Path.Combine (directory, assemblyName + ".dll");
+			var assemblyDef = AssemblyDefinition.CreateAssembly (
+				new AssemblyNameDefinition (assemblyName, new Version (1, 0, 0, 0)),
+				assemblyName,
+				ModuleKind.Dll);
+
+			var resource = new EmbeddedResource (resourceName, ManifestResourceAttributes.Public, content);
+			assemblyDef.MainModule.Resources.Add (resource);
+			assemblyDef.Write (asmPath);
+
+			return asmPath;
+		}
+
+		[Test]
+		public void PathTraversal_IsRejected ()
+		{
+			var tmpdir = Cache.CreateTemporaryDirectory ();
+			// Resource name "__monotouch_content_.._sEvil.txt" unmangles to "../Evil.txt" (path traversal)
+			var assemblyPath = CreateAssemblyWithResource (tmpdir, "TestTraversal", "__monotouch_content_.._sEvil.txt", new byte [] { 0x41 });
+
+			var task = CreateTask<UnpackLibraryResources> ();
+			task.Prefix = "monotouch";
+			task.IntermediateOutputPath = Path.Combine (tmpdir, "intermediate");
+			task.ReferencedLibraries = new [] { new TaskItem (assemblyPath) };
+			task.TargetFrameworkDirectory = Array.Empty<TaskItem> ();
+
+			ExecuteTask (task, expectedErrorCount: 1);
+
+			Assert.That (Engine.Logger.ErrorEvents [0].Message, Does.Contain ("would extract to"));
+			Assert.That (Engine.Logger.ErrorEvents [0].Message, Does.Contain ("outside"));
+
+			// Verify the file was NOT written outside the target directory
+			var escapedPath = Path.Combine (tmpdir, "intermediate", "unpack", "TestTraversal", "Evil.txt");
+			Assert.That (File.Exists (escapedPath), Is.False, "File should not have been extracted outside target directory");
+		}
+
+		[Test]
+		public void ValidResource_IsExtracted ()
+		{
+			var tmpdir = Cache.CreateTemporaryDirectory ();
+			// Resource name "__monotouch_content_sub_sfile.txt" unmangles to "sub/file.txt" (valid path)
+			var assemblyPath = CreateAssemblyWithResource (tmpdir, "TestValid", "__monotouch_content_sub_sfile.txt", new byte [] { 0x41 });
+
+			var task = CreateTask<UnpackLibraryResources> ();
+			task.Prefix = "monotouch";
+			task.IntermediateOutputPath = Path.Combine (tmpdir, "intermediate");
+			task.ReferencedLibraries = new [] { new TaskItem (assemblyPath) };
+			task.TargetFrameworkDirectory = Array.Empty<TaskItem> ();
+
+			ExecuteTask (task, expectedErrorCount: 0);
+
+			var extractedPath = Path.Combine (tmpdir, "intermediate", "unpack", "TestValid", "content", "sub", "file.txt");
+			Assert.That (File.Exists (extractedPath), Is.True, $"File should have been extracted to {extractedPath}");
+		}
+	}
+}

tests/msbuild/Xamarin.MacDev.Tasks.Tests/Xamarin.MacDev.Tasks.Tests.csproj

@@ -45,6 +45,7 @@
     <PackageReference Include="Microsoft.Build.Tasks.Core" Version="$(MicrosoftBuildTasksCorePackageVersion)" />
     <PackageReference Include="Microsoft.Build.Utilities.Core" Version="$(MicrosoftBuildUtilitiesCorePackageVersion)" />
     <PackageReference Include="NUnitXml.TestLogger" Version="$(NUnitXmlTestLoggerPackageVersion)" />
+    <PackageReference Include="Mono.Cecil" Version="$(MonoCecilPackageVersion)" />
     <!-- Fix transient dependency issue found by component governance 4.7.0 -> 4.7.2 -->
     <PackageReference Include="System.Drawing.Common" Version="$(SystemDrawingCommonPackageVersion)" />
     <!-- Fix transient dependency issue found by component governance 4.7.0 -> 4.7.1 -->