Add initial fuzz test#196

Open

DavidKorczynski wants to merge2 commits intosybrenstuvel:main

base:main

Choose a base branch

fromDavidKorczynski:fuzz-integration

Open

Add initial fuzz test#196

DavidKorczynski wants to merge2 commits intosybrenstuvel:mainfromDavidKorczynski:fuzz-integration

Conversation

Copy link

DavidKorczynski commentedApr 6, 2022

Hi,

I was wondering if you would like to integrate continuous fuzzing of python-rsa by way of OSS-Fuzz? In this PRgoogle/oss-fuzz#7516 I do exactly that, namely created the necessary logic from an OSS-Fuzz perspective to integrate python-rsa.

This includes developing initial fuzzers as well as integrating into OSS-Fuzz, however, it is preferable to have the fuzzers upstream so I included it in this PR - if you are happy with having the fuzzers here then I will remove them from the OSS-Fuzz repository.

Essentially, OSS-Fuzz is a free service run by Google that performs continuous fuzzing of important open source projects. The only expectation of integrating into OSS-Fuzz is that bugs will be fixed. This is not a "hard" requirement in that no one enforces this and the main point is if bugs are not fixed then it is a waste of resources to run the fuzzers, which we would like to avoid.

If you would like to integrate, the only thing I need is as list of email(s) that will get access to the data produced by OSS-Fuzz, such as bug reports, coverage reports and more stats. Notice the emails affiliated with the project will be public in the OSS-Fuzz repo, as they will be part of a configuration file.

Add initial fuzz test

e4675fa

Initial fuzz test for string operations roundtrip

Copy link

Owner

sybrenstuvel commentedApr 14, 2022

Thanks for the offer! I'm quite curious to see what the fuzz tests can expose.

As for the pull request, I can't accept it as-is. Theatheris module should be listed as optional development dependency, and the test should be gracefully skipped when it cannot be imported.

I'm also curious why the tests simply return on aValueError orOverflowError. IMO this should be documented in a comment above the respectivereturn statements, so that it's clear for anyone reading the code.

Copy link

Author

DavidKorczynski commentedApr 14, 2022

Thanks for letting me know you're interested in fuzzing@sybrenstuvel -- I will look to address the issues you mention!

nit

e56465b

Copy link

Author

DavidKorczynski commentedApr 19, 2022

Theatheris module should be listed as optional development dependency, and the test should be gracefully skipped when it cannot be imported.

Am not sure how you would prefer this. The fuzzer is not meant to be run similar to the other tests in that a fuzzer never really terminates but is meant to be run continuously. I would advice to have the continuous running of it handled by oss-fuzz, and as such it may be better to move it to another directory thantests/? In that case, where would you prefer to have the dependencyatheris listed? Is it here

python-rsa/pyproject.toml

Line 38 in76c0e69

[tool.poetry.dev-dependencies]

I'm also curious why the tests simply return on aValueError orOverflowError. IMO this should be documented in a comment above the respectivereturn statements, so that it's clear for anyone reading the code.

I put in comments in the exception handling now, let me know what you think.