search

gyscos / cursive

A Text User Interface library for the Rust programming language
MIT License
4.26k stars 243 forks source link

fuzzing report #488

Open alexanderkjall opened 4 years ago

alexanderkjall commented 4 years ago

First: is this a bug report? A suggestion? Or asking for help?

Just some information, and a small suggestion.

Problem description

I wrote a small setup to fuzz the display of untrusted strings with cursive: https://github.com/alexanderkjall/cursive/commit/fada64679f1c37eec59da9662404165d2602e4a2

default ncurses backend

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: NulError(0, [0])', /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/ncurses-5.99.0/src/lib.rs:65:29

crossterm backend

No issue found after half an hour of fuzzing.

pancurses-backend

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: NulError(0, [0])', /home/capitol/.cargo/registry/src/github.com-1ecc6299db9ec823/pancurses-0.16.1/src/window.rs:392:47

termion-backend

thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 24, kind: Other, message: "Too many open files" }', /home/capitol/projects/cursive/cursive/src/cursive_runnable.rs:65:64

This panic is in my fuzzing code, so it might be a false positive, but might also be some sort of resource leakage.

blt-backend

no fuzzing done, had a hard time to get this to work on ubuntu and my arch system isn't available right now.

Suggestions

I have not gone over and replicated the issues in each of the underlying libraries and reported issues there yet.

Looking at the open issues in the ncurses library, it looks like there is a lot of security issues open that hasn't been addressed yet: https://github.com/jeaye/ncurses-rs/issues

I would recommend to either switch the default backend to a more safe one, or at least have a warning note about it in the documentation, as cursive lables itself It is designed to be safe and easy to use.

Environment

alexanderkjall commented 4 years ago

link to issues:

ncurses: https://github.com/jeaye/ncurses-rs/issues/196 pancurses: https://github.com/ihalila/pancurses/issues/77

gyscos commented 4 years ago

Thank you very much for this!

I sort of missed the fact that a null byte was valid in a &str - indeed it'll cause some trouble in FFI.

I suppose we should sanitize any input string (in addition to null bytes, we may want to remove some control codes as well).