Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fuzzing report #488

Open
alexanderkjall opened this issue Aug 19, 2020 · 2 comments
Open

fuzzing report #488

alexanderkjall opened this issue Aug 19, 2020 · 2 comments

Comments

@alexanderkjall
Copy link
Contributor

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: alexanderkjall@fada646

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

  • Operating system: linux (ubuntu 20.04)
  • Backend used: all of them
  • Cursive version: from git
@alexanderkjall
Copy link
Contributor Author

link to issues:

ncurses: jeaye/ncurses-rs#196
pancurses: ihalila/pancurses#77

@gyscos
Copy link
Owner

gyscos commented Aug 20, 2020

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).

@alexanderkjall alexanderkjall mentioned this issue Sep 27, 2020
Merged
yasuo-ozu pushed a commit to yasuo-ozu/rusty-man that referenced this issue May 7, 2023
@robinkrahl
Use termion backend for cursive
8db34e3 
With this patch, we replace cursive’s default ncurses backend with the
termion backend.  This has multiple reasons:
- The ncurses backend has safety issues, see [0].
- ncurses requires a pre-installed library and a C compiler, introducing
  additional build dependencies.  Termion is implemented in Rust only.
- ncurses does not work on Windows, while termion works in all terminals
  that support ANSI escape codes.

Per default, the termion backend does not buffer the output which may
cause flickering [1].  Therefore, we also use the
cursive_buffered_backend that buffers the output and fixes the
flickering problem.

[0] gyscos/cursive#488
[1] gyscos/cursive#142
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@gyscos @alexanderkjall