1
0
mirror of https://github.com/vlang/v.git synced 2023-08-10 21:13:21 +03:00

os: add a security advisory for potential TOCTOU risks when using os.is_writable, os.is_executable etc (#15222)

This commit is contained in:
Bastian Buck 2022-07-26 11:02:48 +02:00 committed by GitHub
parent 03b7c76b38
commit 4ab72ccb69
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 64 additions and 0 deletions

View File

@ -3,3 +3,59 @@
`os` provides common OS/platform independent functions for accessing `os` provides common OS/platform independent functions for accessing
command line arguments, reading/writing files, listing folders, command line arguments, reading/writing files, listing folders,
handling processes etc. 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()

View File

@ -395,6 +395,8 @@ pub fn exists(path string) bool {
} }
// is_executable returns `true` if `path` is executable. // 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 { pub fn is_executable(path string) bool {
$if windows { $if windows {
// Note: https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/access-waccess?view=vs-2019 // 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. // 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] [manualfree]
pub fn is_writable(path string) bool { pub fn is_writable(path string) bool {
$if windows { $if windows {
@ -434,6 +438,8 @@ pub fn is_writable(path string) bool {
} }
// is_readable returns `true` if `path` is readable. // 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] [manualfree]
pub fn is_readable(path string) bool { pub fn is_readable(path string) bool {
$if windows { $if windows {
@ -723,6 +729,8 @@ pub fn is_dir(path string) bool {
} }
// is_link returns a boolean indicating whether `path` is a link. // 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 { pub fn is_link(path string) bool {
$if windows { $if windows {
path_ := path.replace('/', '\\') path_ := path.replace('/', '\\')