From f56003e28854291005a514056cd282a2129308c6 Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sun, 14 Jun 2020 02:05:56 +0200 Subject: [PATCH] Fixed: Performance of symbolic link detection and infinite recursion --- .../SymlinkResolverFixture.cs | 64 ++++++++++ .../Disk/SymbolicLinkResolver.cs | 119 ++++++++++++------ 2 files changed, 148 insertions(+), 35 deletions(-) create mode 100644 src/NzbDrone.Mono.Test/DiskProviderTests/SymlinkResolverFixture.cs diff --git a/src/NzbDrone.Mono.Test/DiskProviderTests/SymlinkResolverFixture.cs b/src/NzbDrone.Mono.Test/DiskProviderTests/SymlinkResolverFixture.cs new file mode 100644 index 000000000..d6d783556 --- /dev/null +++ b/src/NzbDrone.Mono.Test/DiskProviderTests/SymlinkResolverFixture.cs @@ -0,0 +1,64 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Text; +using System.Threading.Tasks; +using FluentAssertions; +using Mono.Posix; +using Mono.Unix; +using NUnit.Framework; +using NzbDrone.Mono.Disk; +using NzbDrone.Test.Common; + +namespace NzbDrone.Mono.Test.DiskProviderTests +{ + [TestFixture] + [Platform("Mono")] + public class SymbolicLinkResolverFixture : TestBase + { + public SymbolicLinkResolverFixture() + { + MonoOnly(); + } + + [Test] + public void should_follow_nested_symlinks() + { + var rootDir = GetTempFilePath(); + var tempDir1 = Path.Combine(rootDir, "dir1"); + var tempDir2 = Path.Combine(rootDir, "dir2"); + var subDir1 = Path.Combine(tempDir1, "subdir1"); + var file1 = Path.Combine(tempDir2, "file1"); + var file2 = Path.Combine(tempDir2, "file2"); + + Directory.CreateDirectory(tempDir1); + Directory.CreateDirectory(tempDir2); + File.WriteAllText(file2, "test"); + + new UnixSymbolicLinkInfo(subDir1).CreateSymbolicLinkTo("../dir2"); + new UnixSymbolicLinkInfo(file1).CreateSymbolicLinkTo("file2"); + + var realPath = Subject.GetCompleteRealPath(Path.Combine(subDir1, "file1")); + + realPath.Should().Be(file2); + } + + [Test] + public void should_throw_on_infinite_loop() + { + var rootDir = GetTempFilePath(); + var tempDir1 = Path.Combine(rootDir, "dir1"); + var subDir1 = Path.Combine(tempDir1, "subdir1"); + var file1 = Path.Combine(tempDir1, "file1"); + + Directory.CreateDirectory(tempDir1); + + new UnixSymbolicLinkInfo(subDir1).CreateSymbolicLinkTo("../../dir1/subdir1/baddir"); + + var realPath = Subject.GetCompleteRealPath(file1); + + realPath.Should().Be(file1); + } + } +} diff --git a/src/NzbDrone.Mono/Disk/SymbolicLinkResolver.cs b/src/NzbDrone.Mono/Disk/SymbolicLinkResolver.cs index 28edae7ca..6077fafa1 100644 --- a/src/NzbDrone.Mono/Disk/SymbolicLinkResolver.cs +++ b/src/NzbDrone.Mono/Disk/SymbolicLinkResolver.cs @@ -1,4 +1,6 @@ using System; +using System.Drawing; +using System.Net; using System.Text; using Mono.Unix; using Mono.Unix.Native; @@ -11,8 +13,6 @@ namespace NzbDrone.Mono.Disk string GetCompleteRealPath(string path); } - // Mono's own implementation doesn't handle exceptions very well. - // All of this code was copied from mono with minor changes. public class SymbolicLinkResolver : ISymbolicLinkResolver { private readonly Logger _logger; @@ -28,34 +28,27 @@ namespace NzbDrone.Mono.Disk try { - string[] dirs; - int lastIndex; - GetPathComponents(path, out dirs, out lastIndex); + var realPath = path; + for (var links = 0; links < 32; links++) + { + var wasSymLink = TryFollowFirstSymbolicLink(ref realPath); + if (!wasSymLink) + { + return realPath; + } + } - var realPath = new StringBuilder(); - if (dirs.Length > 0) - { - var dir = UnixPath.IsPathRooted(path) ? "/" : ""; - dir += dirs[0]; - realPath.Append(GetRealPath(dir)); - } - for (var i = 1; i < lastIndex; ++i) - { - realPath.Append("/").Append(dirs[i]); - var realSubPath = GetRealPath(realPath.ToString()); - realPath.Remove(0, realPath.Length); - realPath.Append(realSubPath); - } - return realPath.ToString(); + var ex = new UnixIOException(Errno.ELOOP); + _logger.Warn("Failed to check for symlinks in the path {0}: {1}", path, ex.Message); + return path; } catch (Exception ex) { - _logger.Debug(ex, string.Format("Failed to check for symlinks in the path {0}", path)); + _logger.Debug(ex, "Failed to check for symlinks in the path {0}", path); return path; } } - private static void GetPathComponents(string path, out string[] components, out int lastIndex) { var dirs = path.Split(UnixPath.DirectorySeparatorChar); @@ -87,23 +80,77 @@ namespace NzbDrone.Mono.Disk lastIndex = target; } - public string GetRealPath(string path) + private bool TryFollowFirstSymbolicLink(ref string path) { - do + string[] dirs; + int lastIndex; + GetPathComponents(path, out dirs, out lastIndex); + + if (lastIndex == 0) { - var link = UnixPath.TryReadLink(path); + return false; + } - if (link == null) + var realPath = ""; + + for (var i = 0; i < lastIndex; ++i) + { + if (i != 0 || UnixPath.IsPathRooted(path)) { - var errno = Stdlib.GetLastError(); - if (errno != Errno.EINVAL) - { - _logger.Trace("Checking path {0} for symlink returned error {1}, assuming it's not a symlink.", path, errno); - } - - return path; + realPath = string.Concat(realPath, UnixPath.DirectorySeparatorChar, dirs[i]); + } + else + { + realPath = string.Concat(realPath, dirs[i]); } + var pathValid = TryFollowSymbolicLink(ref realPath, out var wasSymLink); + + if (!pathValid || wasSymLink) + { + // If the path does not exist, or it was a symlink then we need to concat the remaining dir components and start over (or return) + var count = lastIndex - i - 1; + if (count > 0) + { + realPath = string.Concat(realPath, UnixPath.DirectorySeparatorChar, string.Join(UnixPath.DirectorySeparatorChar.ToString(), dirs, i + 1, lastIndex - i - 1)); + } + path = realPath; + return pathValid; + } + } + + return false; + } + + private bool TryFollowSymbolicLink(ref string path, out bool wasSymLink) + { + if (!UnixFileSystemInfo.TryGetFileSystemEntry(path, out var fsentry) || !fsentry.Exists) + { + wasSymLink = false; + return false; + } + + if (!fsentry.IsSymbolicLink) + { + wasSymLink = false; + return true; + } + + var link = UnixPath.TryReadLink(path); + + if (link == null) + { + var errno = Stdlib.GetLastError(); + if (errno != Errno.EINVAL) + { + _logger.Trace("Checking path {0} for symlink returned error {1}, assuming it's not a symlink.", path, errno); + } + + wasSymLink = true; + return false; + } + else + { if (UnixPath.IsPathRooted(link)) { path = link; @@ -113,8 +160,10 @@ namespace NzbDrone.Mono.Disk path = UnixPath.GetDirectoryName(path) + UnixPath.DirectorySeparatorChar + link; path = UnixPath.GetCanonicalPath(path); } - } while (true); - } + wasSymLink = true; + return true; + } + } } }