From 629e67f10ddc9976307cc295efec15ac9aa45d20 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 22 Aug 2022 12:35:43 -0700 Subject: [PATCH 1/3] Skip directory symlink recursion on TarFile archive creation --- .../src/System/Formats/Tar/TarFile.cs | 39 +++++++++++- .../TarFile.CreateFromDirectory.File.Tests.cs | 60 +++++++++++++++++++ ...ile.CreateFromDirectoryAsync.File.Tests.cs | 60 +++++++++++++++++++ 3 files changed, 157 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs index 33d971bfc5605..205cbe86d3c34 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.IO; +using System.IO.Enumeration; using System.Threading; using System.Threading.Tasks; @@ -278,12 +279,20 @@ private static void CreateFromDirectoryInternal(string sourceDirectoryName, Stre DirectoryInfo di = new(sourceDirectoryName); string basePath = GetBasePathForCreateFromDirectory(di, includeBaseDirectory); + bool skipBaseDirRecursion = false; if (includeBaseDirectory) { writer.WriteEntry(di.FullName, GetEntryNameForBaseDirectory(di.Name)); + skipBaseDirRecursion = (di.Attributes & FileAttributes.ReparsePoint) != 0; } - foreach (FileSystemInfo file in di.EnumerateFileSystemInfos("*", SearchOption.AllDirectories)) + if (skipBaseDirRecursion) + { + // The base directory is a symlink, do not recurse into it + return; + } + + foreach (FileSystemInfo file in GetFileSystemEnumerationForCreation(sourceDirectoryName)) { writer.WriteEntry(file.FullName, GetEntryNameForFileSystemInfo(file, basePath.Length)); } @@ -325,18 +334,44 @@ private static async Task CreateFromDirectoryInternalAsync(string sourceDirector DirectoryInfo di = new(sourceDirectoryName); string basePath = GetBasePathForCreateFromDirectory(di, includeBaseDirectory); + bool skipBaseDirRecursion = false; if (includeBaseDirectory) { await writer.WriteEntryAsync(di.FullName, GetEntryNameForBaseDirectory(di.Name), cancellationToken).ConfigureAwait(false); + skipBaseDirRecursion = (di.Attributes & FileAttributes.ReparsePoint) != 0; } - foreach (FileSystemInfo file in di.EnumerateFileSystemInfos("*", SearchOption.AllDirectories)) + if (skipBaseDirRecursion) + { + // The base directory is a symlink, do not recurse into it + return; + } + + foreach (FileSystemInfo file in GetFileSystemEnumerationForCreation(sourceDirectoryName)) { await writer.WriteEntryAsync(file.FullName, GetEntryNameForFileSystemInfo(file, basePath.Length), cancellationToken).ConfigureAwait(false); } } } + // Generates a recursive enumeration of the filesystem entries inside the specified source directory, while + // making sure that directory symlinks do not get recursed. + private static IEnumerable GetFileSystemEnumerationForCreation(string sourceDirectoryName) + { + return new FileSystemEnumerable( + directory: sourceDirectoryName, + transform: (ref FileSystemEntry entry) => entry.ToFileSystemInfo(), + options: new EnumerationOptions() + { + RecurseSubdirectories = true + }) + { + ShouldRecursePredicate = IsDirectorySymlink + }; + + static bool IsDirectorySymlink(ref FileSystemEntry entry) => entry.IsDirectory && (entry.Attributes & FileAttributes.ReparsePoint) == 0; + } + // Determines what should be the base path for all the entries when creating an archive. private static string GetBasePathForCreateFromDirectory(DirectoryInfo di, bool includeBaseDirectory) => includeBaseDirectory && di.Parent != null ? di.Parent.FullName : di.FullName; diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs index 75a3495d94e5e..bda0aa139f955 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs @@ -193,5 +193,65 @@ public void IncludeAllSegmentsOfPath(bool includeBaseDirectory) Assert.Null(reader.GetNextEntry()); } + + [Fact] + public void SkipRecursionIntoDirectorySymlinks() + { + using TempDirectory root = new TempDirectory(); + + string destinationArchive = Path.Join(root.Path, "destination.tar"); + + string externalDirectory = Path.Join(root.Path, "externalDirectory"); + Directory.CreateDirectory(externalDirectory); + + File.Create(Path.Join(externalDirectory, "file.txt")).Dispose(); + + string sourceDirectoryName = Path.Join(root.Path, "baseDirectory"); + Directory.CreateDirectory(sourceDirectoryName); + + string subDirectory = Path.Join(sourceDirectoryName, "subDirectory"); + Directory.CreateSymbolicLink(subDirectory, externalDirectory); // Should not recurse here + + TarFile.CreateFromDirectory(sourceDirectoryName, destinationArchive, includeBaseDirectory: false); + + using FileStream archiveStream = File.OpenRead(destinationArchive); + using (TarReader reader = new(archiveStream, leaveOpen: false)) + { + TarEntry entry = reader.GetNextEntry(); + Assert.NotNull(entry); + Assert.Equal("subDirectory/", entry.Name); + + Assert.Null(reader.GetNextEntry()); // file.txt should not be found + } + } + + [Fact] + public void SkipRecursionIntoBaseDirectorySymlink() + { + using TempDirectory root = new TempDirectory(); + + string destinationArchive = Path.Join(root.Path, "destination.tar"); + + string externalDirectory = Path.Join(root.Path, "externalDirectory"); + Directory.CreateDirectory(externalDirectory); + + string subDirectory = Path.Join(externalDirectory, "subDirectory"); + Directory.CreateDirectory(subDirectory); + + string sourceDirectoryName = Path.Join(root.Path, "baseDirectory"); + Directory.CreateSymbolicLink(sourceDirectoryName, externalDirectory); + + TarFile.CreateFromDirectory(sourceDirectoryName, destinationArchive, includeBaseDirectory: true); // Base directory is a symlink, do not recurse + + using FileStream archiveStream = File.OpenRead(destinationArchive); + using (TarReader reader = new(archiveStream, leaveOpen: false)) + { + TarEntry entry = reader.GetNextEntry(); + Assert.NotNull(entry); + Assert.Equal("baseDirectory/", entry.Name); + + Assert.Null(reader.GetNextEntry()); + } + } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs index c9b4377cd0328..0ef03bbdd2050 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs @@ -237,5 +237,65 @@ public async Task IncludeAllSegmentsOfPath_Async(bool includeBaseDirectory) } } } + + [Fact] + public async Task SkipRecursionIntoDirectorySymlinksAsync() + { + using TempDirectory root = new TempDirectory(); + + string destinationArchive = Path.Join(root.Path, "destination.tar"); + + string externalDirectory = Path.Join(root.Path, "externalDirectory"); + Directory.CreateDirectory(externalDirectory); + + File.Create(Path.Join(externalDirectory, "file.txt")).Dispose(); + + string sourceDirectoryName = Path.Join(root.Path, "baseDirectory"); + Directory.CreateDirectory(sourceDirectoryName); + + string subDirectory = Path.Join(sourceDirectoryName, "subDirectory"); + Directory.CreateSymbolicLink(subDirectory, externalDirectory); // Should not recurse here + + await TarFile.CreateFromDirectoryAsync(sourceDirectoryName, destinationArchive, includeBaseDirectory: false); + + await using FileStream archiveStream = File.OpenRead(destinationArchive); + await using (TarReader reader = new(archiveStream, leaveOpen: false)) + { + TarEntry entry = await reader.GetNextEntryAsync(); + Assert.NotNull(entry); + Assert.Equal("subDirectory/", entry.Name); + + Assert.Null(await reader.GetNextEntryAsync()); // file.txt should not be found + } + } + + [Fact] + public async Task SkipRecursionIntoBaseDirectorySymlinkAsync() + { + using TempDirectory root = new TempDirectory(); + + string destinationArchive = Path.Join(root.Path, "destination.tar"); + + string externalDirectory = Path.Join(root.Path, "externalDirectory"); + Directory.CreateDirectory(externalDirectory); + + string subDirectory = Path.Join(externalDirectory, "subDirectory"); + Directory.CreateDirectory(subDirectory); + + string sourceDirectoryName = Path.Join(root.Path, "baseDirectory"); + Directory.CreateSymbolicLink(sourceDirectoryName, externalDirectory); + + await TarFile.CreateFromDirectoryAsync(sourceDirectoryName, destinationArchive, includeBaseDirectory: true); // Base directory is a symlink, do not recurse + + await using FileStream archiveStream = File.OpenRead(destinationArchive); + await using (TarReader reader = new(archiveStream, leaveOpen: false)) + { + TarEntry entry = await reader.GetNextEntryAsync(); + Assert.NotNull(entry); + Assert.Equal("baseDirectory/", entry.Name); + + Assert.Null(await reader.GetNextEntryAsync()); // subDirectory should not be found + } + } } } From 36e879141044c628ce4137ade7078b74834f253b Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 22 Aug 2022 14:07:22 -0700 Subject: [PATCH 2/3] Add symlink verification --- .../tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs | 2 ++ .../TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs index bda0aa139f955..fbbafb8c71e27 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs @@ -220,6 +220,7 @@ public void SkipRecursionIntoDirectorySymlinks() TarEntry entry = reader.GetNextEntry(); Assert.NotNull(entry); Assert.Equal("subDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); Assert.Null(reader.GetNextEntry()); // file.txt should not be found } @@ -249,6 +250,7 @@ public void SkipRecursionIntoBaseDirectorySymlink() TarEntry entry = reader.GetNextEntry(); Assert.NotNull(entry); Assert.Equal("baseDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); Assert.Null(reader.GetNextEntry()); } diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs index 0ef03bbdd2050..4c21a659ba844 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs @@ -264,6 +264,7 @@ public async Task SkipRecursionIntoDirectorySymlinksAsync() TarEntry entry = await reader.GetNextEntryAsync(); Assert.NotNull(entry); Assert.Equal("subDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); Assert.Null(await reader.GetNextEntryAsync()); // file.txt should not be found } @@ -293,6 +294,7 @@ public async Task SkipRecursionIntoBaseDirectorySymlinkAsync() TarEntry entry = await reader.GetNextEntryAsync(); Assert.NotNull(entry); Assert.Equal("baseDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); Assert.Null(await reader.GetNextEntryAsync()); // subDirectory should not be found } From 5da8dd98c203d10935503be75a4d9bf9737f61e7 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 22 Aug 2022 15:20:13 -0700 Subject: [PATCH 3/3] Address suggestions by danmoseley --- .../src/System/Formats/Tar/TarFile.cs | 4 +-- .../TarFile.CreateFromDirectory.File.Tests.cs | 30 +++++++++---------- ...ile.CreateFromDirectoryAsync.File.Tests.cs | 30 +++++++++---------- 3 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs index 205cbe86d3c34..d57c29850ea22 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs @@ -366,10 +366,10 @@ private static IEnumerable GetFileSystemEnumerationForCreation(s RecurseSubdirectories = true }) { - ShouldRecursePredicate = IsDirectorySymlink + ShouldRecursePredicate = IsNotADirectorySymlink }; - static bool IsDirectorySymlink(ref FileSystemEntry entry) => entry.IsDirectory && (entry.Attributes & FileAttributes.ReparsePoint) == 0; + static bool IsNotADirectorySymlink(ref FileSystemEntry entry) => entry.IsDirectory && (entry.Attributes & FileAttributes.ReparsePoint) == 0; } // Determines what should be the base path for all the entries when creating an archive. diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs index fbbafb8c71e27..388e9639bd295 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectory.File.Tests.cs @@ -215,15 +215,14 @@ public void SkipRecursionIntoDirectorySymlinks() TarFile.CreateFromDirectory(sourceDirectoryName, destinationArchive, includeBaseDirectory: false); using FileStream archiveStream = File.OpenRead(destinationArchive); - using (TarReader reader = new(archiveStream, leaveOpen: false)) - { - TarEntry entry = reader.GetNextEntry(); - Assert.NotNull(entry); - Assert.Equal("subDirectory/", entry.Name); - Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + using TarReader reader = new(archiveStream, leaveOpen: false); - Assert.Null(reader.GetNextEntry()); // file.txt should not be found - } + TarEntry entry = reader.GetNextEntry(); + Assert.NotNull(entry); + Assert.Equal("subDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + + Assert.Null(reader.GetNextEntry()); // file.txt should not be found } [Fact] @@ -245,15 +244,14 @@ public void SkipRecursionIntoBaseDirectorySymlink() TarFile.CreateFromDirectory(sourceDirectoryName, destinationArchive, includeBaseDirectory: true); // Base directory is a symlink, do not recurse using FileStream archiveStream = File.OpenRead(destinationArchive); - using (TarReader reader = new(archiveStream, leaveOpen: false)) - { - TarEntry entry = reader.GetNextEntry(); - Assert.NotNull(entry); - Assert.Equal("baseDirectory/", entry.Name); - Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + using TarReader reader = new(archiveStream, leaveOpen: false); - Assert.Null(reader.GetNextEntry()); - } + TarEntry entry = reader.GetNextEntry(); + Assert.NotNull(entry); + Assert.Equal("baseDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + + Assert.Null(reader.GetNextEntry()); } } } diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs index 4c21a659ba844..bb1d8ff7cc0cc 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.CreateFromDirectoryAsync.File.Tests.cs @@ -259,15 +259,14 @@ public async Task SkipRecursionIntoDirectorySymlinksAsync() await TarFile.CreateFromDirectoryAsync(sourceDirectoryName, destinationArchive, includeBaseDirectory: false); await using FileStream archiveStream = File.OpenRead(destinationArchive); - await using (TarReader reader = new(archiveStream, leaveOpen: false)) - { - TarEntry entry = await reader.GetNextEntryAsync(); - Assert.NotNull(entry); - Assert.Equal("subDirectory/", entry.Name); - Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + await using TarReader reader = new(archiveStream, leaveOpen: false); - Assert.Null(await reader.GetNextEntryAsync()); // file.txt should not be found - } + TarEntry entry = await reader.GetNextEntryAsync(); + Assert.NotNull(entry); + Assert.Equal("subDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + + Assert.Null(await reader.GetNextEntryAsync()); // file.txt should not be found } [Fact] @@ -289,15 +288,14 @@ public async Task SkipRecursionIntoBaseDirectorySymlinkAsync() await TarFile.CreateFromDirectoryAsync(sourceDirectoryName, destinationArchive, includeBaseDirectory: true); // Base directory is a symlink, do not recurse await using FileStream archiveStream = File.OpenRead(destinationArchive); - await using (TarReader reader = new(archiveStream, leaveOpen: false)) - { - TarEntry entry = await reader.GetNextEntryAsync(); - Assert.NotNull(entry); - Assert.Equal("baseDirectory/", entry.Name); - Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + await using TarReader reader = new(archiveStream, leaveOpen: false); - Assert.Null(await reader.GetNextEntryAsync()); // subDirectory should not be found - } + TarEntry entry = await reader.GetNextEntryAsync(); + Assert.NotNull(entry); + Assert.Equal("baseDirectory/", entry.Name); + Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType); + + Assert.Null(await reader.GetNextEntryAsync()); // subDirectory should not be found } } }