alacritty / vte

Parser for virtual terminal emulators
https://docs.rs/vte/
Apache License 2.0
242 stars 56 forks source link

Infinite loop in ParamsIter #77

Closed Gaelan closed 2 years ago

Gaelan commented 2 years ago

I've been doing some fuzzing of vte, and discovered that the input b"\x1bP;;;:::::;;:::::;;;;;;;;;;;;;0;;::;;p\x1b" causes ParamsIter to loop forever.

I'm trying to get my head around the escape code format enough to troubleshoot more usefully, but maybe you can figure out what's going on before I can.

chrisduerr commented 2 years ago

A more minimal repro would be printf "\e[::::::::::::::::::::::::::::::::x\e". This will work for all escapes that accept subparameters. I'll send a patch in a second, thanks for the issue.

Did you notice any other issues while fuzzing? I've fuzzed VTE a while ago before adding subparameter support, so it would make sense if that's the only area with issues.

Gaelan commented 2 years ago

Nope - I fuzzed it for a while, finding nothing, then added the subparameter support (i.e. actually iterating over the Params), and it found this pretty quickly. As far as I can tell there isn't a way to keep fuzzing while ignoring a known issue, so there might be more bugs there.

chrisduerr commented 2 years ago

https://github.com/alacritty/vte/pull/78 should fix this. So if you decide to fuzz some more please do let me know if you find anything else.

Gaelan commented 2 years ago

Will do. Happy to do a PR with the fuzzing infra as well if that'd be helpful.

chrisduerr commented 2 years ago

Will do. Happy to do a PR with the fuzzing infra as well if that'd be helpful.

I'm not sure I'd want that to live in-tree. Last time I did this there were a couple different possible approaches and the fuzzing itself ultimately covered pretty much everything in a couple of minutes. So the payoff isn't that great other than making it easier for people to run it that have never fuzzed Rust before.

Since all regular contributors to VTE shouldn't have a problem with fuzzing Rust, it's not really worth it. Especially because major changes to VTE are rather rare. But I appreciate the suggestion.

Though I am curious what you used for fuzzing, just because I haven't done it in a couple months.

Gaelan commented 2 years ago

Bit more fuzzing doesn't seem to be finding anything more.

Though I am curious what you used for fuzzing, just because I haven't done it in a couple months.

Just used https://github.com/rust-fuzz/cargo-fuzz - the Rust Fuzz Book has some good getting-started documentation. It's very easy.

chrisduerr commented 2 years ago

Ah okay, pretty sure that's also what I settled on last time.