Differential fuzzing of pulldown-cmark and commonmark.js

[mgeisler]

This adds a new fuzzer which will run random Markdown texts through both pulldown-cmark and commonmark.js. The output from commonmark.js is turned into Events and the two are compared.

The bundled commonmark.js and commonmark.min.js files were generated by running

npm run build

at revision 20b52e5 of https://github.com/commonmark/commonmark.js/. The output differs slightly from what we see on https://spec.commonmark.org/dingus/ so I’m not sure what version is deployed there.

[mgeisler]

Hey there! This is the fuzzer I've been using to find

• Parsing inline code span with a single newline loses the newline #655
• Strong with nested emphasis parses as "**" #657
• Parsing "\\\n\n" loses the backslash #658
• Parsing "<!e -hhl" gives text, not HTML #659
• "__!_!__" parsed as strong #660
• Parsing inline code with "\n " loses both newline and space #661
• HTML block loses initial indentation #662

I don't think it makes sense to merge this as-is since it keeps finding more corner cases. Instead of me opening 10+ issues, I hope the fuzzer can be a useful tool for the maintainers of this crate.

I run it with this command:

cargo fuzz run commonmark_js -- -only_ascii=1

[Martin1887]

The dingus deployed version is the latest release (0.30), and so it should be the version used here.

In addition, commonmark.js code should be inside a new directoy inside third_party directory including its license. And, if I'm not wrong, only the minified version is used, right? The another one should be deleted then.

Thanks for your contribution!

[mgeisler]

The dingus deployed version is the latest release (0.30), and so it should be the version used here.

Good point, let me change the JavaScript to a released version to have a stable reference.

I'll also move it to the third_party directory.

[Martin1887]

Any news here? I also think that creating issues reported by the fuzzer is not bad, though a new tag fuzzed_finding would help to identify them, since usually they are less-priority bugs.

[mgeisler]

Any news here? I also think that creating issues reported by the fuzzer is not bad, though a new tag fuzzed_finding would help to identify them, since usually they are less-priority bugs.

No news, I didn't do the cleanups yet. Let me take a look and do that now.

[mgeisler]

Hi @Martin1887, I moved things around and cleaned up the PR a little. I found that I could use https://lib.rs/rquickjs to run the JavaScript and only pay the setup cost once per fuzzer run. This brings a ~5x speedup on my machine compared to before (from about 30 exec/s to around 150 exec/s).

When I run the fuzzer, it normally finds a parsing difference after a few thousand attempts.

also think that creating issues reported by the fuzzer is not bad, though a new tag fuzzed_finding would help to identify them, since usually they are less-priority bugs.

I'm glad you like the issues, I'll keep reporting them then 😄 A tag sounds good so that one can look for common patterns.

[Martin1887]

Great! I will review more carefully the changes and merge the pull request.

The first finding I think can be fixed is the big number of new dependencies: they should be put inside dev-dependencies to avoid compiling them in the final binary and also avoiding the increase of vulnerabilities surface.

Thanks!

[mgeisler]

Great! I will review more carefully the changes and merge the pull request.

Thanks for reviewing it!

The first finding I think can be fixed is the big number of new dependencies: they should be put inside dev-dependencies to avoid compiling them in the final binary and also avoiding the increase of vulnerabilities surface.

The dependencies are only for the fuzzers, so in a sense, they're already dev-dependencies 😄

The anyhow and pretty_assertions dependencies could be removed. I only added pretty_assertions since it shows a nice diff when the fuzzer finds a mismatch. It looks like this in my terminal:

[Martin1887]

My bad, in the GitHub's file diff I didn't notice that it was a different Cargo.toml file.

I have reviewed all changes and they seem right, I only have updated quick-xml to the latest version, released the past week.

However, in ARM the fuzzer fails with the following error in rquickjs-sys:

I will try to test it tomorrow in x86.

Thanks!

[mgeisler]

However, in ARM the fuzzer fails with the following error in rquickjs-sys:

I will try to test it tomorrow in x86.

Oh, interesting! I've only tested it on my Linux machine. I don't know the rquickjs dependency well — it was suggested to me as an easy way to get JavaScript code running and indeed it works nicely (on x86 at least).

[Martin1887]

I have tried it in x86_64 and it works, so I have created the following issue and the pull request can be merged.

DelSkayn/rquickjs#169

Thanks.