Skip to content

Soundness: Stack Buffer Overread in error case #128

Description

@Manishearth

Note

This finding was identified during an agentic unsafe Rust code review performed by Gemini AI, followed by human review and verification.

The Issue

In src/unix.rs and src/wasi.rs, the internal helper function with_description allocates a 1024-byte stack buffer ([0u8; 1024]) and calls strerror_r. If strerror_r returns ERANGE (indicating the OS error message exceeds 1024 bytes), the function checks the return code but does not exit early. Instead, execution proceeds directly to calling libc::strlen(buf.as_ptr()).

rust-errno/src/unix.rs

Lines 32 to 45 in 8209c18

let c_str = unsafe {
let rc = strerror_r(err.0, buf.as_mut_ptr() as *mut _, buf.len() as size_t);
if rc != 0 {
// Handle negative return codes for compatibility with glibc < 2.13
let fm_err = match rc < 0 {
true => errno(),
false => Errno(rc),
};
if fm_err != Errno(libc::ERANGE) {
return callback(Err(fm_err));
}
}
let c_str_len = strlen(buf.as_ptr() as *const _);
&buf[..c_str_len]

According to the POSIX strerror_r specification, the function is not guaranteed to null-terminate the target buffer if it fails with an error such as ERANGE. On target operating systems or libc implementations where the buffer is filled with non-zero bytes without a trailing null terminator, executing strlen scans past the end of the 1024-byte allocation, causing an immediate stack-buffer overread into adjacent stack frame memory.

Note

The full audit report below also contains additional minor findings (such as missing safety comments or undocumented FFI assumptions) that are probably worth fixing as well but not the primary goal of this issue. The audit report has not been human-reviewed, it may contain misleading claims.

Full Gemini Codebase Audit Report Appendix

Unsafe Rust Review: errno (v0_3)

Overall Safety Assessment

The errno crate is a small utility crate that provides a cross-platform interface to the thread-local errno (or GetLastError on Windows) variable.
It contains unsafe blocks for FFI boundaries, dereferencing thread-local pointers, and unchecked UTF-8 conversions.
While the crate is generally well-structured and has a low density of unsafe code, it suffers from a critical soundness issue on Unix-like and WASI platforms where a stack-buffer overread could occur if strerror_r fails with ERANGE and does not null-terminate the buffer.
Additionally, the crate completely lacks safety comments (// SAFETY:) or safety documentation (# Safety), violating standard safety review guidelines.

Critical Findings

Stack-buffer overread on ERANGE in unix.rs and wasi.rs 🔴 🚨

  • Severity: 🔴 High
  • Threat Vector: 🚨 Untrusted Input
  • Bug Type: Stack Buffer Overread

In src/unix.rs:32 and src/wasi.rs:32, with_description allocates a stack-based buffer of size 1024:

let mut buf = [0u8; 1024];

It then calls the platform's strerror_r function:

let rc = strerror_r(err.0, buf.as_mut_ptr() as *mut _, buf.len() as size_t);

If strerror_r returns ERANGE (meaning the error message is too large to fit in 1024 bytes), the code checks rc but does not return. Instead, it proceeds to call libc::strlen:

let c_str_len = strlen(buf.as_ptr() as *const _);

However, the POSIX standard does not guarantee that strerror_r will null-terminate the buffer if it returns an error like ERANGE. On some target operating systems or libc versions, the buffer may be filled entirely with non-null bytes without a null terminator. Calling strlen on such a buffer results in a stack-buffer overread (undefined behavior).

Although standard operating system error messages are typically short, the error code err.0 is an arbitrary user-controlled i32, and OS-specific or locale-specific messages could theoretically trigger this condition.

To fix this, the crate should manually ensure the buffer is null-terminated (e.g. by setting buf[1023] = 0), or it should return an error immediately if ERANGE is returned.

Fishy Findings

None.

Missing Safety Comments

All unsafe blocks in the crate are missing // SAFETY: comments. The following are the proposed safety comments for each site.

1. src/unix.rs:23 and src/wasi.rs:23 (from_utf8_lossy) 🔴

        // SAFETY:
        // `error.valid_up_to()` returns the index in `input` up to which the bytes
        // are valid UTF-8 (AXIOM: standard library documentation for `Utf8Error::valid_up_to`).
        // Therefore, the subslice `&input[..error.valid_up_to()]` contains only valid UTF-8.
        // This satisfies the safety contract of `str::from_utf8_unchecked`.

2. src/unix.rs:32 and src/wasi.rs:32 (with_description strerror_r call) 🔴

Assuming the ERANGE bug has been resolved by either aborting on ERANGE or enforcing null-termination:

    // SAFETY:
    // - `strerror_r` is called with a pointer to `buf` and `buf.len()`.
    //   `buf` is a valid stack-allocated array of 1024 bytes (TYPE FACT: local array).
    //   This ensures the pointer is valid for writes of `buf.len()` bytes.
    // - On success (`rc == 0`), `strerror_r` guarantees that the buffer contains a
    //   null-terminated string (AXIOM: POSIX strerror_r specification).
    // - `strlen` is called on the pointer to `buf`, which is valid and null-terminated
    //   (on success), so it returns the length of the string without reading out of bounds.
    // - The returned slice `&buf[..c_str_len]` is within the bounds of `buf` and is valid for the lifetime
    //   of the borrow, which does not escape the scope of this function.

3. src/unix.rs:53 & src/unix.rs:57 and src/wasi.rs:49 & src/wasi.rs:53 (errno and set_errno) 🔴

    // SAFETY:
    // `errno_location()` (or `__errno_location()`) returns a pointer to the thread-local
    // `errno` variable (AXIOM: standard Unix/libc interface). This pointer is guaranteed
    // to be non-null, aligned, and valid for reads/writes for the duration of the current thread.

4. src/windows.rs:37 (from_utf16_lossy) 🔴

    // SAFETY:
    // `output[..output_len]` is guaranteed to be valid UTF-8 because:
    // - It is populated solely by `c.encode_utf8`, which writes valid UTF-8 sequences
    //   of length `c_len` (AXIOM: `char::encode_utf8` contract).
    // - We increment `output_len` by exactly the number of bytes written (`c_len`).
    // - We ensure `c_len <= output.len() - output_len` before calling `encode_utf8`,
    //   preventing out-of-bounds writes.

5. src/windows.rs:50 (with_description FormatMessageW call) 🔴

    // SAFETY:
    // - `FormatMessageW` is called with `buf.as_mut_ptr()` and `buf.len() as u32`.
    //   `buf` is a stack-allocated array of 2048 `u16`s, which is valid for writes
    //   of `buf.len()` characters (TYPE FACT: local array).
    // - If `res != 0`, `res` represents the number of characters written, which is
    //   guaranteed by `FormatMessageW` to not exceed `buf.len()`.
    // - The slice `&buf[..res as usize]` is therefore within bounds and valid.

6. src/windows.rs:76 and src/windows.rs:80 (errno and set_errno) 🔴

    // SAFETY:
    // Calling `GetLastError` and `SetLastError` are standard Windows API FFI calls
    // with no safety preconditions. They are thread-safe and always valid to call.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions