SerenityOS / serenity

The Serenity Operating System 🐞
https://serenityos.org
BSD 2-Clause "Simplified" License
30.71k stars 3.19k forks source link

[ShellPosix] DoS attack vulnerability in reduce_start #17939

Open hghwng opened 1 year ago

hghwng commented 1 year ago

While fuzzing the "FuzzShellPosix" binary, a DoS attack has been discovered in ParseLexer.cpp::reduce_start. The bug is reproducible with the Base64-encoded input PDwAAAAAAAAAAAAAAAAKCg==. When running the FuzzShellPosix binary with the input, it crashes with the following error message:

FuzzShellPosix: /root/build/serenity/Meta/Lagom/../../AK/Vector.h:148: AK::Vector::VisibleType &AK::Vector<Shell::Posix::Token, 0>::at(size_t) [T = Shell::Posix::Token, inline_capacity = 0]: Assertion `i < m_size' failed.

The root cause is the mis-handling the returned value from Token::maybe_from_state. The returned vector could be empty, thus the access to the the elements (.first() in this case) should be checked first.

https://github.com/SerenityOS/serenity/blob/4f496e97fe553ce392e2bc6860dcb701089c35ee/Userland/Shell/PosixLexer.cpp#L575

alimpfard commented 1 year ago

That's some really strong phrasing for an unchecked access bug in a WIP program found by a fuzzer...

hghwng commented 1 year ago

Thank you for your feedback. I apologize if my phrasing seemed too strong, I was trying to categorize the bug following the convention. Should I omit the categorization in the future?

alimpfard commented 1 year ago

Not really, if you want to do that, go for it.