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 82ed0b4f4..f5d09f9e6 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 <b>TOCTOU</b> vulnerability if used incorrectly. 
+<b>TOCTOU</b> (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 <b>TOCTOU</b> is no longer possible.
+
+
+<b>Example</b>
+<i>Hint</i>: `os.create()` opens a file in write-only mode
+
+<table>
+<tr>
+<td>
+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') ?
+        // <do something with file>
+    f.close()
+}
+```
+</td>
+<td>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()
+```
+</td>
+</tr>
+</table>
+
+<b> Proven affected functions </b></br>
+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 af73a15c1..2a3913cb2 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('/', '\\')
-- 
2.30.2