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

Add fuzzer based on honggfuzz #312

Merged
merged 3 commits into from
Aug 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/fuzzing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Fuzzing

## Installing `honggfuzz`

```
cargo install honggfuzz
```

Install [dependencies](https://github.com/rust-fuzz/honggfuzz-rs#dependencies) for your system.

## Running the fuzzer

Running the fuzzer is as easy as running in the `fuzz` directory.

Choose a target:

These are `[[bin]]` entries in `Cargo.toml`.
List them with `cargo read-manifest | jq '.targets[].name'` from the `fuzz` directory.

Run the fuzzer:

```shell
cd fuzz
cargo hfuzz run <target>
```

After a panic is found, get a stack trace with:

```shell
cargo hfuzz run-debug <target> hfuzz_workspace/<target>/*.fuzz
```

For example, with the `fuzz_parse_sql` target:

```shell
cargo hfuzz run fuzz_parse_sql
cargo hfuzz run-debug fuzz_parse_sql hfuzz_workspace/fuzz_parse_sql/*.fuzz
```
3 changes: 3 additions & 0 deletions fuzz/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
corpus
hfuzz_target
hfuzz_workspace
17 changes: 17 additions & 0 deletions fuzz/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[package]
name = "fuzz"
version = "0.1.0"
edition = "2018"
publish = false

[dependencies]
honggfuzz = "0.5.54"
sqlparser = { path = ".." }

# Prevent this from interfering with workspaces
[workspace]
members = ["."]

[[bin]]
name = "fuzz_parse_sql"
path = "fuzz_targets/fuzz_parse_sql.rs"
12 changes: 12 additions & 0 deletions fuzz/fuzz_targets/fuzz_parse_sql.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
use honggfuzz::fuzz;
use sqlparser::dialect::GenericDialect;
use sqlparser::parser::Parser;

fn main() {
loop {
fuzz!(|data: String| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty cool. Where does the data come from? Is it just random strings? Or does this somehow know how to make valid sql statements?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see some related comments here: #211 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now it is some random bytes, but can be used to find bugs. Optimization can be continued later, refer to: https://www.cockroachlabs.com/blog/sqlsmith-randomized-sql-testing/

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In a past project, we had a harness that could generate random SQL queries and it found many bugs -- such tests are a wonderful way to help database software mature

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed apache/datafusion#913 for DataFusion. Thanks for the pointer @PsiACE

let dialect = GenericDialect {};
let _ = Parser::parse_sql(&dialect, &data);
});
}
}
7 changes: 3 additions & 4 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2202,8 +2202,8 @@ impl<'a> Parser<'a> {
Ok(Query {
with,
body,
limit,
order_by,
limit,
offset,
fetch,
})
Expand Down Expand Up @@ -2358,8 +2358,7 @@ impl<'a> Parser<'a> {
]) // This couldn't possibly be a bad idea
})?
.into_iter()
.filter(|i| i.is_some())
.map(|i| i.unwrap())
.flatten()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct that this is a real bug that was found/fixed with the fuzzer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, just make clippy happy

.collect();

lateral_views.push(LateralView {
Expand Down Expand Up @@ -2414,8 +2413,8 @@ impl<'a> Parser<'a> {
top,
projection,
from,
selection,
lateral_views,
selection,
group_by,
cluster_by,
distribute_by,
Expand Down