From fe04c00b7a6dee96bb85dc7819572853f691f270 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Fri, 24 Apr 2026 13:52:25 -0500 Subject: [PATCH] [xabt] Validate extracted zip entry paths in `ExtractJarsFromAar` This is just for correctness; there's no security enforcement here. Add `IsUnderDirectory` check at both extraction points (jars and annotations) to silently skip zip entries whose resolved path escapes the target directory. Logs a debug message when an entry is skipped. Add `ExtractJarsFromAarTests` with cases for jar traversal, annotations traversal, and a valid jar entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Tasks/ExtractJarsFromAar.cs | 13 +++ .../Tasks/ExtractJarsFromAarTests.cs | 109 ++++++++++++++++++ 2 files changed, 122 insertions(+) create mode 100644 src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/ExtractJarsFromAarTests.cs diff --git a/src/Xamarin.Android.Build.Tasks/Tasks/ExtractJarsFromAar.cs b/src/Xamarin.Android.Build.Tasks/Tasks/ExtractJarsFromAar.cs index 651dd3c9bff..f5fc112a8a0 100644 --- a/src/Xamarin.Android.Build.Tasks/Tasks/ExtractJarsFromAar.cs +++ b/src/Xamarin.Android.Build.Tasks/Tasks/ExtractJarsFromAar.cs @@ -45,6 +45,8 @@ public override bool RunTask () var fileName = Path.GetFileName (entryFullName); if (string.Equals (fileName, "annotations.zip", StringComparison.OrdinalIgnoreCase)) { var path = Path.GetFullPath (Path.Combine (annotationOutputDirectory, entryFullName)); + if (!IsUnderDirectory (path, annotationOutputDirectory, entryFullName, library)) + continue; Extract (entry, memoryStream, path); annotations.Add (path); } else if (!entryFullName.EndsWith (".jar", StringComparison.OrdinalIgnoreCase)) { @@ -53,6 +55,8 @@ public override bool RunTask () continue; } else { var path = Path.GetFullPath (Path.Combine (jarOutputDirectory, entryFullName)); + if (!IsUnderDirectory (path, jarOutputDirectory, entryFullName, library)) + continue; Extract (entry, memoryStream, path); jars.Add (path); } @@ -68,6 +72,15 @@ public override bool RunTask () return !Log.HasLoggedErrors; } + bool IsUnderDirectory (string resolvedPath, string targetDirectory, string entryName, string archivePath) + { + var normalizedDir = Path.GetFullPath (targetDirectory) + Path.DirectorySeparatorChar; + if (resolvedPath.StartsWith (normalizedDir, StringComparison.OrdinalIgnoreCase)) + return true; + Log.LogDebugMessage ($"Skipping archive entry '{entryName}' in '{archivePath}': resolves outside target directory."); + return false; + } + static void Extract (ZipEntry entry, MemoryStream stream, string destination) { stream.SetLength (0); //Reuse the stream diff --git a/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/ExtractJarsFromAarTests.cs b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/ExtractJarsFromAarTests.cs new file mode 100644 index 00000000000..e089ccd9dc4 --- /dev/null +++ b/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Tasks/ExtractJarsFromAarTests.cs @@ -0,0 +1,109 @@ +using System.Collections.Generic; +using System.IO; +using System.IO.Compression; +using Microsoft.Build.Framework; +using NUnit.Framework; +using Xamarin.Android.Tasks; + +namespace Xamarin.Android.Build.Tests +{ + [TestFixture] + [Parallelizable (ParallelScope.Self)] + public class ExtractJarsFromAarTests : BaseTest + { + List? errors; + List? messages; + MockBuildEngine? engine; + string? path; + + [SetUp] + public void Setup () + { + engine = new MockBuildEngine (TestContext.Out, + errors: errors = new List (), + messages: messages = new List ()); + + path = Path.Combine (Root, "temp", TestName); + TestOutputDirectories [TestContext.CurrentContext.Test.ID] = path; + Directory.CreateDirectory (path); + } + + [Test] + public void PathTraversalInJarEntry () + { + var aarPath = CreateAarWithEntry ("../../relative.jar"); + var jarOutputDir = Path.Combine (path, "jars"); + var annotationOutputDir = Path.Combine (path, "annotations"); + Directory.CreateDirectory (jarOutputDir); + Directory.CreateDirectory (annotationOutputDir); + + var task = new ExtractJarsFromAar { + BuildEngine = engine, + OutputJarsDirectory = jarOutputDir, + OutputAnnotationsDirectory = annotationOutputDir, + Libraries = [aarPath], + }; + + Assert.IsTrue (task.Execute (), "Task should succeed, skipping the traversal entry."); + Assert.IsEmpty (errors, "No errors should be logged."); + + // Verify no file was written outside the target directory + var escapedPath = Path.GetFullPath (Path.Combine (path, "relative.jar")); + Assert.IsFalse (File.Exists (escapedPath), "File should not be written outside target directory."); + } + + [Test] + public void PathTraversalInAnnotationsEntry () + { + var aarPath = CreateAarWithEntry ("../../annotations.zip"); + var jarOutputDir = Path.Combine (path, "jars"); + var annotationOutputDir = Path.Combine (path, "annotations"); + Directory.CreateDirectory (jarOutputDir); + Directory.CreateDirectory (annotationOutputDir); + + var task = new ExtractJarsFromAar { + BuildEngine = engine, + OutputJarsDirectory = jarOutputDir, + OutputAnnotationsDirectory = annotationOutputDir, + Libraries = [aarPath], + }; + + Assert.IsTrue (task.Execute (), "Task should succeed, skipping the traversal entry."); + Assert.IsEmpty (errors, "No errors should be logged."); + } + + [Test] + public void ValidJarEntry () + { + var aarPath = CreateAarWithEntry ("libs/helper.jar"); + var jarOutputDir = Path.Combine (path, "jars"); + var annotationOutputDir = Path.Combine (path, "annotations"); + Directory.CreateDirectory (jarOutputDir); + Directory.CreateDirectory (annotationOutputDir); + + var task = new ExtractJarsFromAar { + BuildEngine = engine, + OutputJarsDirectory = jarOutputDir, + OutputAnnotationsDirectory = annotationOutputDir, + Libraries = [aarPath], + }; + + Assert.IsTrue (task.Execute (), "Task should succeed for valid entry."); + Assert.IsEmpty (errors, "No errors should be logged."); + } + + string CreateAarWithEntry (string entryName) + { + var aarPath = Path.Combine (path, "test.aar"); + using (var stream = new FileStream (aarPath, FileMode.Create)) + using (var archive = new ZipArchive (stream, ZipArchiveMode.Create)) { + var entry = archive.CreateEntry (entryName); + using (var entryStream = entry.Open ()) + using (var writer = new StreamWriter (entryStream)) { + writer.Write ("dummy content"); + } + } + return aarPath; + } + } +}