Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions lib/tinykvm/linux/system_calls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2893,10 +2893,10 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls)
path = cpu.machine().memcstring(vpath, PATH_MAX);
// Check if the path is a symlink
if (cpu.machine().fds().resolve_symlink(path)) {
// Copy the resolved path to the guest, respecting guest buffer size.
const size_t copied = std::min<size_t>(path.size(), g_size);
cpu.machine().copy_to_guest(g_buffer, path.c_str(), copied);
regs.rax = copied;
// Copy the resolved path to the guest
const size_t copy_size = std::min(path.size(), size_t(g_size));
cpu.machine().copy_to_guest(g_buffer, path.c_str(), copy_size);
regs.rax = copy_size;
}
else if (UNLIKELY(!cpu.machine().fds().is_readable_path(path))) {
// Pretend the path is not a link
Expand All @@ -2906,13 +2906,17 @@ void Machine::setup_linux_system_calls(bool unsafe_syscalls)
// Translate from vfd when fd != AT_FDCWD
if (vfd != AT_FDCWD)
fd = cpu.machine().fds().translate(vfd);
// Read link target into host buffer, then copy to guest memory.
// Path is in allow-list
const size_t h_size = std::min(size_t(g_size), size_t(PATH_MAX));
char host_buffer[PATH_MAX];
const size_t host_size = std::min<size_t>(g_size, sizeof(host_buffer));
const ssize_t bytes = readlinkat(fd, path.c_str(), host_buffer, host_size);
if (bytes >= 0) {
cpu.machine().copy_to_guest(g_buffer, host_buffer, size_t(bytes));
regs.rax = bytes;
// readlinkat() returns -1 on failure, so storing that in
// regs.rax (unsigned) could wrap. Preserves the original intent to store
// the length in regs.rax (on success) but avoid a segfault if -1
// (which becomes 0xFFFFFFFFFFFFFFFF) and then is sent to copy_to_guest() ...
const ssize_t host_result = readlinkat(fd, path.c_str(), host_buffer, h_size);
if (host_result >= 0) {
cpu.machine().copy_to_guest(g_buffer, host_buffer, host_result);
regs.rax = host_result;
} else {
regs.rax = -errno;
}
Expand Down
30 changes: 30 additions & 0 deletions tests/unit/basic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,33 @@ int main() {
// and the data matched 'Hello World!'.
REQUIRE(output_is_hello_world);
}

TEST_CASE("readlinkat failure path does not overflow copy", "[Output][Syscall]")
{
const auto binary = build_and_load(R"M(
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <unistd.h>

int main() {
char buf[32];
errno = 0;
long rc = readlinkat(AT_FDCWD, "/proc/self/fd/999999", buf, sizeof(buf));
if (rc != -1) return 21;
if (errno <= 0) return 22;
return 0;
}
)M");

tinykvm::Machine machine{binary, {.max_mem = MAX_MEMORY}};
machine.fds().set_open_readable_callback(
[&](std::string& path) -> bool {
(void)path;
return true;
});
machine.setup_linux({"readlinkat-check"}, env);
machine.run(2.0f);

REQUIRE(machine.return_value() == 0);
}
Loading