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.

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

Panic when parsing fuzzed code #1768

Closed
5225225 opened this issue Jan 2, 2022 · 4 comments
Closed

Panic when parsing fuzzed code #1768

5225225 opened this issue Jan 2, 2022 · 4 comments
Assignees
Labels
bug Something isn't working lexer Issues surrounding the lexer
Milestone

Comments

@5225225
Copy link

5225225 commented Jan 2, 2022

Describe the bug

The code should not panic.

To Reproduce

fn main() {
    let data = b"\n(\nx\n)\n";
    drop(boa::parse(data, true));
}

Expected behavior

It shouldn't panic, and it should parse correctly (as whatever it's meant to parse as, not sure.)

Build environment:

  • OS: Arch Linux
  • Target triple: x86_64-unknown-linux-gnu (I think, couldn't find how to print it and I don't think it's terribly important here)
  • Rustc version: rustc 1.59.0-nightly (cfa3fe5af 2021-12-31)
@5225225 5225225 added the bug Something isn't working label Jan 2, 2022
@Razican
Copy link
Member

Razican commented Jan 2, 2022

One question, though. Why are you calling drop() on the parsed result? That could cause a double drop maybe?

@5225225
Copy link
Author

5225225 commented Jan 2, 2022

drop just consumes the input. (It's not magic, and since it's not unsafe it can't cause a double drop).

If you look at https://doc.rust-lang.org/std/mem/fn.drop.html it says it's literally defined as

pub fn drop<T>(_x: T) { }

I do it to silence the unused variable warning that you otherwise get from the Result.

@RageKnify RageKnify added the lexer Issues surrounding the lexer label Jan 2, 2022
@RageKnify
Copy link
Member

RageKnify commented Jan 2, 2022

I've tried to diagnose it and I'm pretty sure it's a bug in the lexer, I think its because there is only a \n after the closing parenthesis.
It tries to peek for a token after the ) to check if it is =>, which would mean it is an arrow function. But in this case there is no other token ahead of it and while trying to fill the lexer's buffer something goes wrong.

@RageKnify RageKnify added parser Issues surrounding the parser lexer Issues surrounding the lexer and removed lexer Issues surrounding the lexer parser Issues surrounding the parser labels Jan 2, 2022
@RageKnify
Copy link
Member

The BufferedLexer's write_index is "overflowing"
peeked contains ['\n', '(', '\n', 'x', '\n', ')', '\n']

The line trying to peek is here

TokenKind::Punctuator(Punctuator::CloseParen) => {
// Need to check if the token after the close paren is an arrow, if so then this is an ArrowFunction
// otherwise it is an expression of the form (b).
if let Some(t) = cursor.peek(3)? {
if t.kind() == &TokenKind::Punctuator(Punctuator::Arrow)

I have the peeking visualized below:

\n   (  \n   x  \n   )  \n  ?
0    0   1   1   2   2  3   3

This seems to be the max size we expected, the ? becomes None during the fill function, but right afterwards it increments write_index, which goes back to 0 and it fails the assertion:

self.write_index = (self.write_index + 1) % PEEK_BUF_SIZE;
debug_assert_ne!(
self.read_index, self.write_index,
"we reached the read index with the write index"
);

@Razican Razican added this to the v0.14.0 milestone Jan 31, 2022
@Razican Razican self-assigned this Feb 5, 2022
Razican added a commit that referenced this issue Feb 5, 2022
@Razican Razican linked a pull request Feb 5, 2022 that will close this issue
Razican added a commit that referenced this issue Feb 7, 2022
@bors bors bot closed this as completed in 4a11ca1 Feb 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lexer Issues surrounding the lexer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants