ncurses is unsound and unmaintained

[lewismosciski]

The crate's repository is archived and unmaintained, but it retains several unsound APIs that expose uninitialized memory.

Overview

The ncurses-rs library contains a set of critical memory safety vulnerabilities affecting 11 functions. These functions improperly use Vec::set_len() and String::set_len() when handling string reads, leading to:

Primary Security Issues

Uninitialized Memory Exposure - Sets Vec/String length to capacity instead of actual data read

Vulnerability Pattern

All affected functions follow the same flawed pattern:

// Flawed implementation pattern
unsafe {
    // Call underlying C function to read data
    let ret = ll::some_read_function(...);
    
    let capacity = s.capacity();
    match s.iter().position(|x| *x == 0) {  // or s.find('\0')
        Some(index) => s.set_len(index),
        None => s.set_len(capacity),  // Exposes uninitialized memory
    }
    
    ret
}

Example

pub fn inchnstr(s: &mut Vec<chtype>, n: i32) -> i32
{

  s.clear();
  s.reserve(n as usize);
  unsafe
  {
    let ret = ll::inchnstr(s.as_ptr(), n);

    let capacity = s.capacity();
    match s.iter().position(|x| *x == 0)
    {
      Some(index) => s.set_len(index as usize),
      None => s.set_len(capacity),
    }

    ret
  }
}

POC:

fn main() {
    initscr();

    let test_str = "0123456789";
    let _ = mvaddstr(5, 5, test_str);
    let _ = refresh();

    let mut buffer: Vec<chtype> = Vec::with_capacity(100);

    let _ = mv(5, 5);
    inchnstr(&mut buffer, test_str.len() as i32);

    endwin();
    println!("Buffer length: {}", buffer.len());
    println!("Buffer capacity: {}", buffer.capacity());
    println!("Buffer contents (first 20): {:?}", buffer);
}

Output:

Buffer's incorrect length (should be 10): 100
Buffer's capacity: 100
Buffer contents (raw chtype values): [48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0]

[djc]

@jeaye or @leshow would you be able to confirm this issue? Would be nice!

Would be nice to reference some upstream issue here rather than linking a random gist.

[jeaye]

@jeaye or @leshow would you be able to confirm this issue? Would be nice!

I can confirm that ncurses-rs is about as unsafe as Rust can get, since it's a very thin wrapper around a terribly unsafe C API.

Would be nice to reference some upstream issue here rather than linking a random gist.

As the repository is archived, creating an issue is not viable. I'm not sure how you prefer to handle this, but un-archiving just to create an issue, then re-archiving doesn't seem like something which should be needed.

EDIT: The archive announcement, in case it's relevant: https://www.reddit.com/r/rust/comments/1hm847x/ncursesrs_has_been_archived/

[djc]

@jeaye thanks for the quick response!

As the repository is archived, creating an issue is not viable. I'm not sure how you prefer to handle this, but un-archiving just to create an issue, then re-archiving doesn't seem like something which should be needed.

@lewismosciski maybe inline some (maybe more concise) version of your analysis in the description of this PR, and then just point to this PR as the authoritative URL.

[lewismosciski]

@djc Updated! The PR now serves as the authoritative reference.

@jeaye Thanks for the confirmation!

[djc]

You have the repo URL as the advisory issue, which doesn't seem helpful. You also didn't inline the gist contents in this PR's description as I suggested.

[djc]

Thanks!