From 49255506f37ba61514c55b2f8bd6515ba1cae3c3 Mon Sep 17 00:00:00 2001 From: Kaz Kylheku Date: Sat, 30 Jul 2022 12:09:51 -0700 Subject: path-components-safe: tighten /proc check Attacks are possible via /proc//fd/ involving a deleted file, whereby the link target changes from "/path/to/file" to "/path/to/file (deleted)", which can be perpetrated by a different user, not related to process , who has access to perform unlink("/path/to/file"). * stdlib/path-test.tl (safe-abs-path): Perform the pattern check regardless of effective user ID. * tests/018/path-safe.tl: Test cases adjusted. --- stdlib/path-test.tl | 29 ++++++++++++++-------------- tests/018/path-safe.tl | 51 +++++++++++++++++++++----------------------------- 2 files changed, 35 insertions(+), 45 deletions(-) diff --git a/stdlib/path-test.tl b/stdlib/path-test.tl index c0b4ea3e..55b5e842 100644 --- a/stdlib/path-test.tl +++ b/stdlib/path-test.tl @@ -183,21 +183,20 @@ (defun safe-abs-path (comps) (flet ((digstr (s) [all s chr-isdigit])) (let ((safe t)) - (if (zerop (geteuid)) - (when-match ("proc" @(or @(digstr) "self") . @rest) - (path-simplify comps) - (match-case rest - (@(or ("cwd" . @nil) - ("root" . @nil) - ("map_files" . @nil) - ("fd" @(digstr) . @nil)) - (zap safe)) - (("task" @(digstr) . @trest) - (match-case trest - (@(or ("cwd" . @nil) - ("root" . @nil) - ("fd" @(digstr) . @nil)) - (zap safe))))))) + (when-match ("proc" @(or @(digstr) "self") . @rest) + (path-simplify comps) + (match-case rest + (@(or ("cwd" . @nil) + ("root" . @nil) + ("map_files" . @nil) + ("fd" @(digstr) . @nil)) + (zap safe)) + (("task" @(digstr) . @trest) + (match-case trest + (@(or ("cwd" . @nil) + ("root" . @nil) + ("fd" @(digstr) . @nil)) + (zap safe)))))) safe))) (defun path-components-safe (path) diff --git a/tests/018/path-safe.tl b/tests/018/path-safe.tl index 2c86ca3e..77b92321 100644 --- a/tests/018/path-safe.tl +++ b/tests/018/path-safe.tl @@ -81,34 +81,25 @@ (seteuid 0) (rename-path "z" "b/c") -(seteuid 10000) - -(test - (path-components-safe "a") nil) - -(mtest - (path-components-safe "/proc/1") t - (path-components-safe "/proc/1/cwd") :error - (path-components-safe "/proc/self/cwd") t) - -(seteuid 0) -(mtest - (path-components-safe "/proc/1") t - (path-components-safe "/proc/1/fd") t - (path-components-safe "/proc/sys/../1") t - (path-components-safe "/proc/1/cwd") nil - (path-components-safe "/proc/1/cwd/foo") nil - (path-components-safe "/proc/self/cwd") nil - (path-components-safe "/proc/self/cwd/foo") nil - (path-components-safe "/proc/1/root") nil - (path-components-safe "/proc/1/root/foo") nil - (path-components-safe "/proc/1/fd/0") nil - (path-components-safe "/proc/1/fd/0/bar") nil - (path-components-safe "/proc/1/map_files") nil - (path-components-safe "/proc/1/map_files/bar") nil - (path-components-safe "/proc/sys/../1/cwd") nil - (path-components-safe "/proc/1/task/1") t - (path-components-safe "/proc/1/task/1/fd/0") nil - (path-components-safe "/proc/1/task/1/cwd") nil - (path-components-safe "/proc/1/task/1/root") nil) +(each ((uid '(10000 0))) + (mtest + (path-components-safe "a") nil + (path-components-safe "/proc/1") t + (path-components-safe "/proc/1/fd") t + (path-components-safe "/proc/sys/../1") t + (path-components-safe "/proc/1/cwd") nil + (path-components-safe "/proc/1/cwd/foo") nil + (path-components-safe "/proc/self/cwd") nil + (path-components-safe "/proc/self/cwd/foo") nil + (path-components-safe "/proc/1/root") nil + (path-components-safe "/proc/1/root/foo") nil + (path-components-safe "/proc/1/fd/0") nil + (path-components-safe "/proc/1/fd/0/bar") nil + (path-components-safe "/proc/1/map_files") nil + (path-components-safe "/proc/1/map_files/bar") nil + (path-components-safe "/proc/sys/../1/cwd") nil + (path-components-safe "/proc/1/task/1") t + (path-components-safe "/proc/1/task/1/fd/0") nil + (path-components-safe "/proc/1/task/1/cwd") nil + (path-components-safe "/proc/1/task/1/root") nil))1 -- cgit v1.2.3