From 330988c4f0fc9773c00b94e1c95d54d09e72276f 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 | 111 +++++++++++++----- 2 files changed, 143 insertions(+), 32 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 829197522..a190fc1ce 100644 --- a/src/NzbDrone.Mono/Disk/SymbolicLinkResolver.cs +++ b/src/NzbDrone.Mono/Disk/SymbolicLinkResolver.cs @@ -1,5 +1,4 @@ -using System; -using System.Text; +using System; using Mono.Unix; using Mono.Unix.Native; using NLog; @@ -11,8 +10,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; @@ -31,31 +28,23 @@ namespace NzbDrone.Mono.Disk try { - string[] dirs; - int lastIndex; - GetPathComponents(path, out dirs, out lastIndex); - - var realPath = new StringBuilder(); - if (dirs.Length > 0) + var realPath = path; + for (var links = 0; links < 32; links++) { - 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); + var wasSymLink = TryFollowFirstSymbolicLink(ref realPath); + if (!wasSymLink) + { + return realPath; + } } - 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; } } @@ -92,23 +81,79 @@ 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; + } + + var realPath = ""; - if (link == null) + for (var i = 0; i < lastIndex; ++i) + { + if (i != 0 || UnixPath.IsPathRooted(path)) { - var errno = Stdlib.GetLastError(); - if (errno != Errno.EINVAL) + 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) { - _logger.Trace("Checking path {0} for symlink returned error {1}, assuming it's not a symlink.", path, errno); + realPath = string.Concat(realPath, UnixPath.DirectorySeparatorChar, string.Join(UnixPath.DirectorySeparatorChar.ToString(), dirs, i + 1, lastIndex - i - 1)); } - return path; + 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; @@ -118,8 +163,10 @@ namespace NzbDrone.Mono.Disk path = UnixPath.GetDirectoryName(path) + UnixPath.DirectorySeparatorChar + link; path = UnixPath.GetCanonicalPath(path); } + + wasSymLink = true; + return true; } - while (true); } } }