commit 31f2b5ab2bf26bf7c89c30bae896884865561e04
parent 14bc0428c2124fdb34a1962b53309be60e3f5013
Author: illiliti <illiliti@dimension.sh>
Date: Mon, 7 Aug 2023 05:24:14 +0300
os::exec: fix TOCTOU
Signed-off-by: illiliti <illiliti@dimension.sh>
Diffstat:
2 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/os/exec/exec+freebsd.ha b/os/exec/exec+freebsd.ha
@@ -59,15 +59,6 @@ export fn pipe() (io::file, io::file) = {
};
fn open(path: str) (platform_cmd | error) = {
- // TODO: This is racy, rewrite with TOCTOU
- match (rt::access(path, rt::X_OK)) {
- case let err: rt::errno =>
- return errors::errno(err);
- case let b: bool =>
- if (!b) {
- return errors::noaccess;
- };
- };
let fd = match (rt::open(path, rt::O_RDONLY, 0u)) {
case let fd: int =>
yield fd;
@@ -76,6 +67,14 @@ fn open(path: str) (platform_cmd | error) = {
};
let success = false;
defer if (!success) rt::close(fd)!;
+ match (rt::faccessat(fd, "", rt::X_OK, rt::AT_EMPTY_PATH)) {
+ case let err: rt::errno =>
+ return errors::errno(err);
+ case let b: bool =>
+ if (!b) {
+ return errors::noaccess;
+ };
+ };
// Make sure we are not trying to execute anything weird. fstat()
// already dereferences symlinks, so if this is anything other than a
// regular file it cannot be executed.
diff --git a/os/exec/exec+linux.ha b/os/exec/exec+linux.ha
@@ -59,14 +59,6 @@ export fn pipe() (io::file, io::file) = {
};
fn open(path: str) (platform_cmd | error) = {
- match (rt::access(path, rt::X_OK)) {
- case let err: rt::errno =>
- return errors::errno(err);
- case let b: bool =>
- if (!b) {
- return errors::noaccess;
- };
- };
// O_PATH is used because it allows us to use an executable for which we
// have execute permissions, but not read permissions.
let fd = match (rt::open(path, rt::O_PATH, 0u)) {
@@ -77,6 +69,14 @@ fn open(path: str) (platform_cmd | error) = {
};
let success = false;
defer if (!success) rt::close(fd)!;
+ match (rt::faccessat(fd, "", rt::X_OK, rt::AT_EMPTY_PATH)) {
+ case let err: rt::errno =>
+ return errors::errno(err);
+ case let b: bool =>
+ if (!b) {
+ return errors::noaccess;
+ };
+ };
// Make sure we are not trying to execute anything weird. fstat()
// already dereferences symlinks, so if this is anything other than a
// regular file it cannot be executed.