-
Notifications
You must be signed in to change notification settings - Fork 14k
Description
use std::env;
use std::process;
fn main() {
let args: Vec<String> = env::args().collect();
if args.len() != 3 {
eprintln!("Usage: {} <string> <length>", args[0]);
process::exit(1);
}
let buffer = &args[1];
let Ok(length) = args[2].parse::<usize>() else {
eprintln!("Error: length must be a valid integer");
process::exit(1);
};
unsafe {
libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
}
}you can cause undefined behavior. It does get caught by miri:
error: Undefined Behavior: memory access failed: attempting to access 1000 bytes, but got alloc1 which is only 4 bytes from the end of the allocation
--> src/main.rs:21:9
|
21 | libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
But we could leverage the #[diagnostic::*] namespace to annotate functions that have this kind of C API with independent pointer and length, and then lint if the length argument isn't directly derived from buffer. I would go as far as making the lint simple in its analysis of that and check that length is either buffer.length() or std::cmp::max(buffer.len(), length) and trigger otherwise (and ask people to enable it for anything more complex, at least at the start?).
The annotation would likely look something like:
#[diagnostic::c_buffer_length(arg2, arg3)]
pub fn write(arg1: c_int, arg2: *const c_void, arg3: c_int) -> c_int;and the output be something like
error: when calling `write`, `length` must be derived from `buffer` to avoid accidental out-of-bounds access
--> src/main.rs:21:9
|
LL | let buffer = &args[1];
| ------ `buffer` created here
LL | let Ok(length) = args[2].parse::<usize>() else {
| ------ `length` created here, unrelated to `buffer`
..
LL | libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
| --------------- ^^^^^^ must be related to `buffer`
| |
| a pointer to `buffer` was captured here
|
= help: this can cause a bug in the program: `write` could perform an invalid operation, and cause Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: consider using the length of the buffer being passed in directly
|
LL - libc::write(1, buffer.as_ptr() as *const libc::c_void, length);
LL + libc::write(1, buffer.as_ptr() as *const libc::c_void, buffer.length());
|
help: alternatively, if you desire to constrain the buffer up to `length`, then ensure that you account for `length` being longer than the buffer
|
LL | libc::write(1, buffer.as_ptr() as *const libc::c_void, std::cmp::max(length, buffer.length()));
| ++++++++++++++ ++++++++++++++++++
|
This lint would is not a reason not to use miri, just a way to catch a possible mistake earlier.
Inspired by https://nitter.net/filpizlo/status/1984366437390303265