-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
TOB-FUEL-31: Sway parser is crashing on certain inputs #5049
Comments
I'm interested in this issue |
## Description Will partially address #5049 ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] I have updated the documentation where relevant (API docs, the reference, and the Sway book). - [x] I have added tests that prove my fix is effective or that my feature works. - [x] I have added (or requested a maintainer to add) the necessary `Breaking*` or `New Feature` labels where relevant. - [x] I have done my best to ensure that my PR adheres to [the Fuel Labs Code Review Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md). - [x] I have requested a review from the relevant team or maintainers.
The parser still contains the stack overflow panics, but fixing those is going to require a significant rewrite. |
We marked the issue as acknowledged. We plan to fix it in the future, but as stated above, it requires rewriting of the parser. It is not a critical issue and doesn't bring vulnerabilities into users' code. We keep this issue open to track the progress in our backlog. |
Description
A fuzzing campaign for the sway parser revealed several crashes. This finding serves as an umbrella finding to list all the different crashes we observed. The harness for the fuzzer is shown in the following figure.
Figure 31.1: Fuzz harness
The findings are relevant for use cases where untrusted Sway programs are parsed, for example as part of a Sway playground application, similar to the Rust playground.
Stack overflow
The Rust program invoking the parser with the following inputs might panic with the message thread 'fuzz_parse_fuzz::entry' has overflowed its stack.
Figure 31.2: Inputs separated by ----- that cause a stack overflow
Unwrap on None value
The parser panics in emit_error in line 50.
Figure 31.3: Input that causes a panic.
Panic while slicing
The parser panics when calling as_str.
Figure 31.4: Input that causes a panic.
Panic while creating span
The parser panics when calling span.
Figure 31.4: Hex encoded inputs separated by ----- that cause a panic.
Recommendations
Short term, avoid panicking by handling the errors appropriately. For the stack overflows, make sure to avoid recursive calls and instead implement parsing iteratively.
Long term, deploy a fuzzer for the Sway parser. The above fuzz harness can be used together with Trail of Bits’ test-fuzz fuzzer. The instructions for setting it up can be found in the documentation of the project.
The text was updated successfully, but these errors were encountered: