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

Cryptofuzz questions #2386

Closed
guidovranken opened this issue May 7, 2019 · 8 comments
Closed

Cryptofuzz questions #2386

guidovranken opened this issue May 7, 2019 · 8 comments

Comments

@guidovranken
Copy link
Contributor

Grouping of different issues

This is an entirely different bug than this, they don't even abort() at the same line, but somehow they get filed under the same issue.

There can also be cases where the abort() occurs at the same line with different underlying issues. In Cryptofuzz, mismatch detection happens at one central place, where it will always abort in case of a mismatch, irrespective of the cipher or message digest.
The same is true for aborting on ciphertexts that cannot be decrypted.

If grouping occurred at least based on unique stack traces, then I can make some C++ templates that implement a distinct function that calls abort() for each combination of library+operation+cipher/digest.

I would like to:

  • have the system detect unique aborts, and distinctly detect them as fixed, etc, where "unique abort()" means a unique combination of library, operation (eg. encrypt, HMAC, ...), and involved digest/cipher.
  • by extension of the previous point, not miss any bugs. I keep an eye on the crash repository at oss-fuzz.com, but ideally I'd like to get a new email for each bug.

More generally, how can we optimally use ClusterFuzz (that was built with sanitizers in mind, which mostly emit a unique signal/stack trace per unique bug) in conjunction with differential fuzzers (which typically (?) implement an abort() once for multiple internal fatal error states) ? Any thoughts?

Fuzzing multiple OpenSSL versions

Currently supported versions of OpenSSL are 1.0.2 (supported until 2020), 1.0.1 (supported until 11 September 2019), and 1.1.0.

If you have sufficient computing power to spare, I'd personally like to fuzz all three versions in addition to Git master. Each version comprises unique code that is core infrastructure software used in production, after all. (I'd also like to see this for the general OpenSSL fuzzers, but that's not my domain..).

If we do that, it might find things that OpenSSL decides not to fix if it isn't considered a security vulnerability. In that case I will work around it in Cryptofuzz. For example see openssl/openssl#8688

I'd like to know your opinion on this.

KDF fuzzing

KDFs are slow by design. Cryptofuzz supports KDF differential fuzzing but I'd like to use different fuzzing targets for it. It IS possible to fuzz both digests and ciphers (which we are doing now) and KDFs in one binary, but

  • The KDF fuzzing will be a major bottleneck in finding more digest/cipher bugs because it slows down the entire process
  • KDFs take entirely different parameters than digests and ciphers (which all consume key, cleartext, optionally IV and some similar fields), so separating the two entails more effective crossover mutations for both.

Are you OK with creating a separate set of fuzzers for KDFs?

@guidovranken
Copy link
Contributor Author

I've limited the output size for all KDFs and it's much faster now. I've enabled them and in the next build they will be activated.

@Dor1s
Copy link
Contributor

Dor1s commented May 20, 2019

Grouping of different issues

If you could print an assertion failure with different error message (it can be either some hash value or stacktrace based signature), than the grouping should work fine.

Fuzzing multiple OpenSSL versions

Sounds good to me. Would you need to add more fuzz targets to achieve that?

KDF fuzzing

Do you think it still makes sense to fuzz those despite KDF being very slow?

A separate set of fuzzers SGTM though.

We may need to bump the Cryptofuzz project weight so it gets a bit more CPUs allocated, in case we'll get too many fuzz targets in it :)

@guidovranken
Copy link
Contributor Author

If you could print an assertion failure with different error message (it can be either some hash value or stacktrace based signature), than the grouping should work fine.

I already do this.

Expand the stack traces of:

https://oss-fuzz.com/testcase-detail/6216562802950144
https://oss-fuzz.com/testcase-detail/5679541617426432

And you'll see that Cryptofuzz prints different info. They both get identified as issue 14461, but they should be separate issues.

Sounds good to me. Would you need to add more fuzz targets to achieve that?

Yep. Can't link 2 versions of OpenSSL into the same binary.

Do you think it still makes sense to fuzz those despite KDF being very slow?

Initially I was allowing KDF handlers to output up to 10 megabytes of data. I've reduced that to max 1024 bytes. It's now very fast overall. Will still find bugs (if any) like this recent one weidai11/cryptopp@e0b6043

We may need to bump the Cryptofuzz project weight so it gets a bit more CPUs allocated, in case we'll get too many fuzz targets in it :)

The amount of libraries should not affect speed or memory usage significantly.
Each fuzzer iteration, I run 0 - 20 operations, each in a random library. So having just 1 or 100 libraries loaded doesn't matter.

@guidovranken
Copy link
Contributor Author

I already do this.

Expand the stack traces of:

https://oss-fuzz.com/testcase-detail/6216562802950144
https://oss-fuzz.com/testcase-detail/5679541617426432

And you'll see that Cryptofuzz prints different info. They both get identified as issue 14461, but they should be separate issues.

Or do you mean a literal assert()? But I don't see how this would change anything, because it does the same as what Cryptofuzz currently does: print text, then abort().

@Dor1s
Copy link
Contributor

Dor1s commented May 21, 2019

Yeah, I meant if you could print:

Assertion failure: <some unique text here>

then CF would parse it and use as the crash state, instead of parsing the stack trace.

@guidovranken
Copy link
Contributor Author

So Clusterfuzz has a special handler for output before the crash that starts with Assertion failure:?

@Dor1s
Copy link
Contributor

Dor1s commented May 21, 2019

Yes, there is a pretty flexible signature::

assertion failed: YOUR TEXT HERE
assert failure: YOUR TEXT HERE
...

see all combinations in https://github.com/google/clusterfuzz/blob/b5852e8885a80572e7950400649753d270c1cbb1/src/python/crash_analysis/stack_parsing/stack_analyzer.py#L59

@guidovranken
Copy link
Contributor Author

Ah, that's wonderful. Thank you.

I have a PR for OpenSSL 1.0.2 and OpenSSL 1.1.0 but it's finding a bunch of new bugs on the latest corpus so I'll make sure those get fixed first before I submit the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants