From 4ab72ccb6945a66de9398f4cd65d06e5dfe4091b Mon Sep 17 00:00:00 2001 From: Bastian Buck <59334447+bstnbuck@users.noreply.github.com> Date: Tue, 26 Jul 2022 11:02:48 +0200 Subject: [PATCH] os: add a security advisory for potential TOCTOU risks when using `os.is_writable`, `os.is_executable` etc (#15222) --- vlib/os/README.md | 56 +++++++++++++++++++++++++++++++++++++++++++++++ vlib/os/os.c.v | 8 +++++++ 2 files changed, 64 insertions(+) diff --git a/vlib/os/README.md b/vlib/os/README.md index 82ed0b4f40..f5d09f9e6a 100644 --- a/vlib/os/README.md +++ b/vlib/os/README.md @@ -3,3 +3,59 @@ `os` provides common OS/platform independent functions for accessing command line arguments, reading/writing files, listing folders, handling processes etc. + +* * * + + +### Security advice related to TOCTOU attacks + +A few `os` module functions can lead to the TOCTOU vulnerability if used incorrectly. +TOCTOU (Time-of-Check-to-Time-of-Use problem) can occur when a file, folder or similar +is checked for certain specifications (e.g. read, write permissions) and a change is made +afterwards. +In the time between the initial check and the edit, an attacker can then cause damage. +The following example shows an attack strategy on the left and an improved variant on the right +so that TOCTOU is no longer possible. + + +Example +Hint: `os.create()` opens a file in write-only mode + + + + + + +
+Possibility for TOCTOU attack + +```v ignore +if os.is_writable("file"){ + + // >> time to make a quick attack (e.g. symlink /etc/passwd to >file<) << + + mut f := os.create('path/to/file') ? + // + f.close() +} +``` +TOCTOU not possible + +```v ignore +mut f := os.create('path/to/file') or { + println("file not writable") +} + +// >> do someting with file; file is locked << + +f.close() +``` +
+ + Proven affected functions
+The following functions should be used with care and only when used correctly. + +* os.is_readable() +* os.is_writable() +* os.is_executable() +* os.is_link() diff --git a/vlib/os/os.c.v b/vlib/os/os.c.v index af73a15c1c..2a3913cb2b 100644 --- a/vlib/os/os.c.v +++ b/vlib/os/os.c.v @@ -395,6 +395,8 @@ pub fn exists(path string) bool { } // is_executable returns `true` if `path` is executable. +// Warning: `is_executable()` is known to cause a TOCTOU vulnerability when used incorrectly +// (for more information: https://github.com/vlang/v/blob/master/vlib/os/README.md) pub fn is_executable(path string) bool { $if windows { // Note: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/access-waccess?view=vs-2019 @@ -419,6 +421,8 @@ pub fn is_executable(path string) bool { } // is_writable returns `true` if `path` is writable. +// Warning: `is_writable()` is known to cause a TOCTOU vulnerability when used incorrectly +// (for more information: https://github.com/vlang/v/blob/master/vlib/os/README.md) [manualfree] pub fn is_writable(path string) bool { $if windows { @@ -434,6 +438,8 @@ pub fn is_writable(path string) bool { } // is_readable returns `true` if `path` is readable. +// Warning: `is_readable()` is known to cause a TOCTOU vulnerability when used incorrectly +// (for more information: https://github.com/vlang/v/blob/master/vlib/os/README.md) [manualfree] pub fn is_readable(path string) bool { $if windows { @@ -723,6 +729,8 @@ pub fn is_dir(path string) bool { } // is_link returns a boolean indicating whether `path` is a link. +// Warning: `is_link()` is known to cause a TOCTOU vulnerability when used incorrectly +// (for more information: https://github.com/vlang/v/blob/master/vlib/os/README.md) pub fn is_link(path string) bool { $if windows { path_ := path.replace('/', '\\')