rust-lang / rust-lang/rust

Support linting c-like fn calls that take pointer-to-slice and length

Open
#148,664 0 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

A-diagnostics A-lints D-papercut T-compiler
Dominant language
Rust
Stars
119k
Forks
16.1k
PR merge metrics
PR metrics pending

Description

Given code like:

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the Rust Playground example and the Miri diagnostic at src/main.rs:21 to reproduce the out-of-bounds call. Investigate how the proposed #[diagnostic::*] annotation and compiler lint infrastructure could express the pointer-and-length relationship; done would mean an agreed design and implementation that catches unrelated lengths without replacing Miri.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
compilers, security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.