From 9b4839e30b56445b4b9e6b707d28546f83febd62 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 8 Sep 2026 09:12:04 -0500 Subject: [PATCH 1/5] fix(file-manager): resolve symlink-backed rsync targets Resolve symlink-backed destinations before File Manager rsync operations so rsync 3.5.0 does not reject exclusive-share paths. Preserve the existing path boundary and rsync security checks.\n\nLinear: OS-871 --- emhttp/plugins/dynamix/nchan/file_manager | 36 ++++++++++- tests/file-manager-rsync-target-path.php | 73 +++++++++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 tests/file-manager-rsync-target-path.php diff --git a/emhttp/plugins/dynamix/nchan/file_manager b/emhttp/plugins/dynamix/nchan/file_manager index 97c82b97a..897af0128 100755 --- a/emhttp/plugins/dynamix/nchan/file_manager +++ b/emhttp/plugins/dynamix/nchan/file_manager @@ -282,6 +282,38 @@ function validname($name, $real=true) { return in_array($root, ['mnt','boot']) ? $path.'/'.basename($name).(mb_substr($name,-1) == '/' ? '/' : '') : ''; } +function resolve_rsync_path($name) { + // Rsync 3.5.0 rejects untrusted symlink components in operator-supplied paths. + // Resolve existing components physically, then append missing destination components. + $trailing_slash = mb_substr($name, -1) == '/'; + $path = rtrim($name, '/'); + $missing = []; + + while (!file_exists($path) && $path != '/') { + if (is_link($path)) return ''; + $missing[] = basename($path); + $path = dirname($path); + } + + $path = realpath($path); + if ($path === false) return ''; + + while (!empty($missing)) { + $path .= '/'.array_pop($missing); + } + + return $path.($trailing_slash ? '/' : ''); +} + +function rsync_target($name) { + $target = validname($name, false); + if (!$target) return ''; + + $target = resolve_rsync_path($target); + $root = explode('/', $target)[1] ?? ''; + return in_array($root, ['mnt','boot']) ? $target : ''; +} + function update_translation($locale) { global $docroot,$language; $language = []; @@ -456,7 +488,7 @@ while (true) { // start action } else { parse_rsync_progress(null, null, true); // Reset static variables - $target = validname($target, false); + $target = rsync_target($target); if ($target) { $mkpath = isdir($target) ? '--mkpath' : ''; $cmd = "rsync -ahPIX$H $sparse $exist $mkpath --no-inc-recursive --out-format=%f --info=flist0,misc0,stats0,name1,progress2 ".quoted($source)." ".escapeshellarg($target)." > >(stdbuf -o0 tr '\\r' '\\n' >$status) 2>$error & echo \$!"; @@ -500,7 +532,7 @@ while (true) { // start action } else { parse_rsync_progress(null, null, true); // Reset static variables - $target = validname($target, false); + $target = rsync_target($target); if ($target) { $mkpath = isdir($target) ? '--mkpath' : ''; diff --git a/tests/file-manager-rsync-target-path.php b/tests/file-manager-rsync-target-path.php new file mode 100644 index 000000000..30fef510b --- /dev/null +++ b/tests/file-manager-rsync-target-path.php @@ -0,0 +1,73 @@ +#!/usr/bin/env php += $tokenCount || !is_array($tokens[$j]) || $tokens[$j][0] !== T_STRING || $tokens[$j][1] !== $name) continue; + $capturing = true; + } + + if (!$capturing) continue; + + $code .= is_array($token) ? $token[1] : $token; + if (is_string($token)) { + if ($token === '"') { + $inDoubleQuote = !$inDoubleQuote; + } elseif (!$inDoubleQuote && $token === '{') { + $braces++; + } elseif (!$inDoubleQuote && $token === '}' && --$braces === 0) { + break; + } + } + } + + if ($code === '') throw new RuntimeException("Could not extract $name"); + return $code; +} + +function assertSameValue(string $expected, string $actual, string $message): void +{ + if ($expected !== $actual) { + throw new RuntimeException(sprintf('%s Expected %s, got %s.', $message, $expected, $actual)); + } +} + +$fileManager = dirname(__DIR__) . '/emhttp/plugins/dynamix/nchan/file_manager'; +$resolveFunction = extractFunction(file_get_contents($fileManager), 'resolve_rsync_path'); +eval($resolveFunction); + +$root = sys_get_temp_dir() . '/unraid-file-manager-path-' . bin2hex(random_bytes(6)); +$real = "$root/real"; +$link = "$root/link"; + +if (!mkdir($real, 0777, true) || !symlink($real, $link)) { + throw new RuntimeException('Could not create the symlink test fixture.'); +} + +try { + $resolvedReal = realpath($real); + assertSameValue("$resolvedReal/", resolve_rsync_path("$link/"), 'An existing symlink target must resolve physically.'); + assertSameValue("$resolvedReal/new/deep/", resolve_rsync_path("$link/new/deep/"), 'A missing descendant must remain under the resolved parent.'); +} finally { + if (is_link($link)) unlink($link); + if (is_dir($real)) rmdir($real); + if (is_dir($root)) rmdir($root); +} + +echo "File Manager rsync target path regression test passed.\n"; From 08a2e5eb4fa4c9f7b6cb6690ae096fb6a9445e86 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:11:04 -0500 Subject: [PATCH 2/5] test(file-manager): expand rsync target coverage Cover the rsync target adapter and both copy and move call sites. Clean up temporary fixtures when setup fails and document the new helpers.\n\nLinear: OS-871\nReview: #2745 --- emhttp/plugins/dynamix/nchan/file_manager | 12 ++++++++++++ tests/file-manager-rsync-target-path.php | 19 +++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/emhttp/plugins/dynamix/nchan/file_manager b/emhttp/plugins/dynamix/nchan/file_manager index 897af0128..17f89b45c 100755 --- a/emhttp/plugins/dynamix/nchan/file_manager +++ b/emhttp/plugins/dynamix/nchan/file_manager @@ -282,6 +282,12 @@ function validname($name, $real=true) { return in_array($root, ['mnt','boot']) ? $path.'/'.basename($name).(mb_substr($name,-1) == '/' ? '/' : '') : ''; } +/** + * Resolve existing path components for an rsync destination. + * + * @param string $name The validated destination path. + * @return string The resolved path, or an empty string when resolution fails. + */ function resolve_rsync_path($name) { // Rsync 3.5.0 rejects untrusted symlink components in operator-supplied paths. // Resolve existing components physically, then append missing destination components. @@ -305,6 +311,12 @@ function resolve_rsync_path($name) { return $path.($trailing_slash ? '/' : ''); } +/** + * Validate and resolve a destination for an rsync operation. + * + * @param string $name The requested destination path. + * @return string The safe rsync destination, or an empty string when invalid. + */ function rsync_target($name) { $target = validname($name, false); if (!$target) return ''; diff --git a/tests/file-manager-rsync-target-path.php b/tests/file-manager-rsync-target-path.php index 30fef510b..0932074d1 100644 --- a/tests/file-manager-rsync-target-path.php +++ b/tests/file-manager-rsync-target-path.php @@ -49,18 +49,25 @@ function assertSameValue(string $expected, string $actual, string $message): voi } $fileManager = dirname(__DIR__) . '/emhttp/plugins/dynamix/nchan/file_manager'; -$resolveFunction = extractFunction(file_get_contents($fileManager), 'resolve_rsync_path'); -eval($resolveFunction); +$fileManagerSource = file_get_contents($fileManager); +if ($fileManagerSource === false) throw new RuntimeException('Could not read the File Manager source.'); + +foreach (['truepath', 'validname', 'resolve_rsync_path', 'rsync_target'] as $function) { + eval(extractFunction($fileManagerSource, $function)); +} + +assertSameValue('', rsync_target('/tmp/outside/'), 'A target outside the allowed roots must be rejected.'); +assertSameValue('2', (string)substr_count($fileManagerSource, '$target = rsync_target($target);'), 'Copy and move must both use the rsync target adapter.'); $root = sys_get_temp_dir() . '/unraid-file-manager-path-' . bin2hex(random_bytes(6)); $real = "$root/real"; $link = "$root/link"; -if (!mkdir($real, 0777, true) || !symlink($real, $link)) { - throw new RuntimeException('Could not create the symlink test fixture.'); -} - try { + if (!mkdir($real, 0777, true) || !symlink($real, $link)) { + throw new RuntimeException('Could not create the symlink test fixture.'); + } + $resolvedReal = realpath($real); assertSameValue("$resolvedReal/", resolve_rsync_path("$link/"), 'An existing symlink target must resolve physically.'); assertSameValue("$resolvedReal/new/deep/", resolve_rsync_path("$link/new/deep/"), 'A missing descendant must remain under the resolved parent.'); From 58b04cf8dbf30f22a111fbf933571194b43a09b7 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Tue, 8 Sep 2026 14:17:56 -0500 Subject: [PATCH 3/5] test(file-manager): isolate copy and move coverage --- tests/file-manager-rsync-target-path.php | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/file-manager-rsync-target-path.php b/tests/file-manager-rsync-target-path.php index 0932074d1..19f45ed27 100644 --- a/tests/file-manager-rsync-target-path.php +++ b/tests/file-manager-rsync-target-path.php @@ -48,6 +48,11 @@ function assertSameValue(string $expected, string $actual, string $message): voi } } +function assertContainsText(string $needle, string $haystack, string $message): void +{ + if (!str_contains($haystack, $needle)) throw new RuntimeException($message); +} + $fileManager = dirname(__DIR__) . '/emhttp/plugins/dynamix/nchan/file_manager'; $fileManagerSource = file_get_contents($fileManager); if ($fileManagerSource === false) throw new RuntimeException('Could not read the File Manager source.'); @@ -57,7 +62,18 @@ function assertSameValue(string $expected, string $actual, string $message): voi } assertSameValue('', rsync_target('/tmp/outside/'), 'A target outside the allowed roots must be rejected.'); -assertSameValue('2', (string)substr_count($fileManagerSource, '$target = rsync_target($target);'), 'Copy and move must both use the rsync target adapter.'); + +$copyStart = strpos($fileManagerSource, 'case 3: // copy folder'); +$moveStart = strpos($fileManagerSource, 'case 4: // move folder'); +$moveEnd = strpos($fileManagerSource, 'case 11: // change owner', $moveStart ?: 0); +if ($copyStart === false || $moveStart === false || $moveEnd === false || $copyStart >= $moveStart) { + throw new RuntimeException('Could not isolate copy and move operation bodies.'); +} + +$copyBody = substr($fileManagerSource, $copyStart, $moveStart - $copyStart); +$moveBody = substr($fileManagerSource, $moveStart, $moveEnd - $moveStart); +assertContainsText('$target = rsync_target($target);', $copyBody, 'Copy must use the rsync target adapter.'); +assertContainsText('$target = rsync_target($target);', $moveBody, 'Move must use the rsync target adapter.'); $root = sys_get_temp_dir() . '/unraid-file-manager-path-' . bin2hex(random_bytes(6)); $real = "$root/real"; From 501e35c75874ef97aef3ed6227a434e350fc2832 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Wed, 9 Sep 2026 07:34:00 -0500 Subject: [PATCH 4/5] fix(file-manager): reject untrusted rsync symlink targets Limit exclusive-share path translation to verified OS mappings and reject other symlink components before rsync receives a pre-resolved path. This preserves rsync 3.5 symlink protections while retaining exclusive-share support. Linear: OS-872 Backport of OS-871 --- emhttp/plugins/dynamix/nchan/file_manager | 99 +++++++++++++++++++---- tests/file-manager-rsync-target-path.php | 14 +++- 2 files changed, 94 insertions(+), 19 deletions(-) diff --git a/emhttp/plugins/dynamix/nchan/file_manager b/emhttp/plugins/dynamix/nchan/file_manager index 17f89b45c..a96dec665 100755 --- a/emhttp/plugins/dynamix/nchan/file_manager +++ b/emhttp/plugins/dynamix/nchan/file_manager @@ -282,33 +282,102 @@ function validname($name, $real=true) { return in_array($root, ['mnt','boot']) ? $path.'/'.basename($name).(mb_substr($name,-1) == '/' ? '/' : '') : ''; } +function rsync_share_is_exclusive($share) { + global $docroot; + $state = rtrim($docroot ?? '/usr/local/emhttp', '/') . '/state/shares.ini'; + $shares = @parse_ini_file($state, true); + return is_array($shares) && in_array((string)($shares[$share]['exclusive'] ?? ''), ['yes', '1'], true); +} + +/** + * Return the physical path for an OS-managed exclusive-share link. + * + * The link is trusted only when it is an exclusive share, its system location + * matches the link target, and it points to a configured storage device. + * + * @param string $path The symlink path. + * @return string The trusted physical target, or an empty string otherwise. + */ +function rsync_exclusive_share_target($path) { + global $docroot; + if (!preg_match('#^/mnt/(user|user0)/([^/]+)$#', $path, $matches)) return ''; + + $share = $matches[2]; + if (!rsync_share_is_exclusive($share)) return ''; + + $link_target = @readlink($path); + if (!is_string($link_target) || $link_target === '') return ''; + if (mb_substr($link_target, 0, 1) !== '/') $link_target = dirname($path) . '/' . $link_target; + $target = truepath($link_target); + + if (!preg_match('#^/mnt/([^/]+)/([^/]+)$#', $target, $target_parts)) return ''; + if ($target_parts[2] !== $share || in_array($target_parts[1], ['user', 'user0', 'rootshare'], true)) return ''; + + // system.LOCATION is maintained by Unraid and identifies the backing + // storage for an exclusive user share. It prevents a similarly shaped link + // from being treated as the OS-managed mapping. + $location = trim((string)@shell_exec("getfattr --absolute-names --only-values -n system.LOCATION " . escapeshellarg($path) . " 2>/dev/null")); + if (!preg_match('/^[A-Za-z0-9._-]+$/D', $location) || $location !== $target_parts[1]) return ''; + + $disks_state = rtrim($docroot ?? '/usr/local/emhttp', '/') . '/state/disks.ini'; + $disks = @parse_ini_file($disks_state, true); + if (!is_array($disks) || !array_key_exists($target_parts[1], $disks)) return ''; + + return is_dir($target) ? $target : ''; +} + /** - * Resolve existing path components for an rsync destination. + * Resolve a validated destination without following arbitrary symlinks. + * + * Only the OS-managed exclusive-share link is translated to its physical + * backing path. All other symlink components are rejected so the compatibility + * path cannot bypass rsync's symlink policy with a pre-resolved attacker- + * controlled path. * * @param string $name The validated destination path. * @return string The resolved path, or an empty string when resolution fails. */ function resolve_rsync_path($name) { - // Rsync 3.5.0 rejects untrusted symlink components in operator-supplied paths. - // Resolve existing components physically, then append missing destination components. $trailing_slash = mb_substr($name, -1) == '/'; - $path = rtrim($name, '/'); - $missing = []; + $pending = array_values(array_filter(explode('/', trim($name, '/')), 'mb_strlen')); + $resolved = []; + $symlink_count = 0; + + while (!empty($pending)) { + $part = array_shift($pending); + if ($part === '.' || $part === '') continue; + if ($part === '..') { + if (!empty($resolved)) array_pop($resolved); + continue; + } - while (!file_exists($path) && $path != '/') { - if (is_link($path)) return ''; - $missing[] = basename($path); - $path = dirname($path); - } + $candidate = '/' . implode('/', array_merge($resolved, [$part])); + $stat = @lstat($candidate); + if ($stat === false) { + $resolved[] = $part; + foreach ($pending as $remaining) { + if ($remaining !== '') $resolved[] = $remaining; + } + break; + } - $path = realpath($path); - if ($path === false) return ''; + if ((($stat['mode'] ?? 0) & 0170000) === 0120000) { + if (++$symlink_count > 40) return ''; + $target = rsync_exclusive_share_target($candidate); + if ($target === '') return ''; + + // Restart from the absolute trusted target so its components are still + // checked rather than blindly accepting a second symlink. + $resolved = []; + $pending = array_merge(array_values(array_filter(explode('/', trim($target, '/')), 'mb_strlen')), $pending); + continue; + } - while (!empty($missing)) { - $path .= '/'.array_pop($missing); + $resolved[] = $part; } - return $path.($trailing_slash ? '/' : ''); + $path = '/' . implode('/', $resolved); + return $path . ($trailing_slash && $path !== '/' ? '/' : ''); } /** diff --git a/tests/file-manager-rsync-target-path.php b/tests/file-manager-rsync-target-path.php index 19f45ed27..57972f8e9 100644 --- a/tests/file-manager-rsync-target-path.php +++ b/tests/file-manager-rsync-target-path.php @@ -53,14 +53,21 @@ function assertContainsText(string $needle, string $haystack, string $message): if (!str_contains($haystack, $needle)) throw new RuntimeException($message); } +function assertNotContainsText(string $needle, string $haystack, string $message): void +{ + if (str_contains($haystack, $needle)) throw new RuntimeException($message); +} + $fileManager = dirname(__DIR__) . '/emhttp/plugins/dynamix/nchan/file_manager'; $fileManagerSource = file_get_contents($fileManager); if ($fileManagerSource === false) throw new RuntimeException('Could not read the File Manager source.'); -foreach (['truepath', 'validname', 'resolve_rsync_path', 'rsync_target'] as $function) { +foreach (['truepath', 'validname', 'rsync_share_is_exclusive', 'rsync_exclusive_share_target', 'resolve_rsync_path', 'rsync_target'] as $function) { eval(extractFunction($fileManagerSource, $function)); } +assertNotContainsText('realpath(', extractFunction($fileManagerSource, 'resolve_rsync_path'), 'The rsync path resolver must not follow arbitrary symlinks with realpath().'); + assertSameValue('', rsync_target('/tmp/outside/'), 'A target outside the allowed roots must be rejected.'); $copyStart = strpos($fileManagerSource, 'case 3: // copy folder'); @@ -84,9 +91,8 @@ function assertContainsText(string $needle, string $haystack, string $message): throw new RuntimeException('Could not create the symlink test fixture.'); } - $resolvedReal = realpath($real); - assertSameValue("$resolvedReal/", resolve_rsync_path("$link/"), 'An existing symlink target must resolve physically.'); - assertSameValue("$resolvedReal/new/deep/", resolve_rsync_path("$link/new/deep/"), 'A missing descendant must remain under the resolved parent.'); + assertSameValue('', resolve_rsync_path("$link/"), 'An unrecognized symlink destination must be rejected.'); + assertSameValue('', resolve_rsync_path("$link/new/deep/"), 'A missing descendant under an unrecognized symlink must be rejected.'); } finally { if (is_link($link)) unlink($link); if (is_dir($real)) rmdir($real); From d8c5bb3f79f988ea3d4bfae5a4a3fede3f8095a2 Mon Sep 17 00:00:00 2001 From: SimonFair <39065407+SimonFair@users.noreply.github.com> Date: Wed, 9 Sep 2026 07:49:30 -0500 Subject: [PATCH 5/5] fix(file-manager): read exclusive share location from symlink Use getfattr --no-dereference so the exclusive-share validation reads the OS-managed symlink attribute instead of following it to the backing directory. --- emhttp/plugins/dynamix/nchan/file_manager | 2 +- tests/file-manager-rsync-target-path.php | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/emhttp/plugins/dynamix/nchan/file_manager b/emhttp/plugins/dynamix/nchan/file_manager index a96dec665..67d0107f9 100755 --- a/emhttp/plugins/dynamix/nchan/file_manager +++ b/emhttp/plugins/dynamix/nchan/file_manager @@ -316,7 +316,7 @@ function rsync_exclusive_share_target($path) { // system.LOCATION is maintained by Unraid and identifies the backing // storage for an exclusive user share. It prevents a similarly shaped link // from being treated as the OS-managed mapping. - $location = trim((string)@shell_exec("getfattr --absolute-names --only-values -n system.LOCATION " . escapeshellarg($path) . " 2>/dev/null")); + $location = trim((string)@shell_exec("getfattr --no-dereference --absolute-names --only-values -n system.LOCATION " . escapeshellarg($path) . " 2>/dev/null")); if (!preg_match('/^[A-Za-z0-9._-]+$/D', $location) || $location !== $target_parts[1]) return ''; $disks_state = rtrim($docroot ?? '/usr/local/emhttp', '/') . '/state/disks.ini'; diff --git a/tests/file-manager-rsync-target-path.php b/tests/file-manager-rsync-target-path.php index 57972f8e9..d77ceba5c 100644 --- a/tests/file-manager-rsync-target-path.php +++ b/tests/file-manager-rsync-target-path.php @@ -67,6 +67,7 @@ function assertNotContainsText(string $needle, string $haystack, string $message } assertNotContainsText('realpath(', extractFunction($fileManagerSource, 'resolve_rsync_path'), 'The rsync path resolver must not follow arbitrary symlinks with realpath().'); +assertContainsText('--no-dereference', extractFunction($fileManagerSource, 'rsync_exclusive_share_target'), 'The exclusive-share location lookup must inspect the symlink itself.'); assertSameValue('', rsync_target('/tmp/outside/'), 'A target outside the allowed roots must be rejected.');