jmcnamara / XlsxWriter

A Python module for creating Excel XLSX files.
https://xlsxwriter.readthedocs.io
BSD 2-Clause "Simplified" License
3.66k stars 631 forks source link

OSS-Fuzz initial integration #1030

Closed ennamarie19 closed 1 year ago

ennamarie19 commented 1 year ago

Hello, This is a follow-on from my Issue regarding OSS-Fuzz integration. This PR introduces the source for the fuzzer. I would greatly appreciate it being merged in!

I will continue monitoring OSS-Fuzz for bugs and fix any I can in future PRs. John McNamara also has access to the OSS-Fuzz dashboard to review any security-relevant crashes that may come up.

Please let me know if anything else is needed.

Thank you!

ennamarie19 commented 1 year ago

@jmcnamara Apologies, I squashed the commits and reopened a new, clean PR

jmcnamara commented 1 year ago

Apologies, I squashed the commits and reopened a new, clean PR

No problem.

ennamarie19 commented 1 year ago

@jmcnamara Does everything look good to go?

jmcnamara commented 1 year ago

@ennamarie19 There are 2 open review questions.

ennamarie19 commented 1 year ago

I can’t seem to find these comments, could you please point me to them?On Nov 7, 2023, at 4:23 PM, John McNamara @.***> wrote: @ennamarie19 There are 2 open review questions.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: @.***>

jmcnamara commented 1 year ago

I can’t seem to find these comments,

You should see comments like the following on the landing page for the PR and inline in the code:

Image

capuanob commented 1 year ago

@jmcnamara Hey John, I think you need to click 'finish review' to move them from 'Pending' to public

capuanob commented 1 year ago

@jmcnamara The Google copyright was added to follow the rule laid out here. As for the second question, the reason this works is because $SRC is defined in the parent Docker image defined by Google. The accompanying Dockerfile that will build the container nightly also was defined to copy the fuzzing corpus from /dev/ to $SRC/corpus with the following line RUN cp -r XlsxWriter/dev/fuzzing/corpus $SRC/corpus

jmcnamara commented 1 year ago

Hey John, I think you need to click 'finish review' to move them from 'Pending' to public

My bad. :-(

The Google copyright was added to follow the rule laid out here.

That says:

Please include copyright headers for all files checked in to oss-fuzz

The code under review isn't being checked into oss-fuzz though. Or are you also submitting it to oss-fuzz?

The accompanying Dockerfile that will build the container nightly also was defined to copy the fuzzing corpus from /dev/ to $SRC/corpus with the following line RUN cp -r XlsxWriter/dev/fuzzing/corpus $SRC/corpus

Could you share a link to the Dockerfile.

capuanob commented 1 year ago

Hey John, I think you need to click 'finish review' to move them from 'Pending' to public

My bad. :-(

The Google copyright was added to follow the rule laid out here.

That says:

Please include copyright headers for all files checked in to oss-fuzz

The code under review isn't being checked into oss-fuzz though. Or are you also submitting it to oss-fuzz?

The accompanying Dockerfile that will build the container nightly also was defined to copy the fuzzing corpus from /dev/ to $SRC/corpus with the following line RUN cp -r XlsxWriter/dev/fuzzing/corpus $SRC/corpus

Could you share a link to the Dockerfile.

Fair point, I assumed that applied to any and all files related to OSSFuzz. But no, the code will live entirely here. ClusterFuzz will pull from the master branch nightly (including the harness) to perform its fuzz-testing. I'll remove it and see if that's cool with them.

The Dockerfile has yet to be pushed to OSSFuzz, since we wanted to integrate into upstream first. You can see our progress here

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

ennamarie19 commented 1 year ago

Hey John, I think you need to click 'finish review' to move them from 'Pending' to public

My bad. :-(

The Google copyright was added to follow the rule laid out here.

That says:

Please include copyright headers for all files checked in to oss-fuzz

The code under review isn't being checked into oss-fuzz though. Or are you also submitting it to oss-fuzz?

The accompanying Dockerfile that will build the container nightly also was defined to copy the fuzzing corpus from /dev/ to $SRC/corpus with the following line RUN cp -r XlsxWriter/dev/fuzzing/corpus $SRC/corpus

Could you share a link to the Dockerfile.

Fair point, I assumed that applied to any and all files related to OSSFuzz. But no, the code will live entirely here. ClusterFuzz will pull from the master branch nightly (including the harness) to perform its fuzz-testing. I'll remove it and see if that's cool with them.

The Dockerfile has yet to be pushed to OSSFuzz, since we wanted to integrate into upstream first. You can see our progress here

@jmcnamara copyright headers have been removed.

jmcnamara commented 1 year ago

Merged. Thanks for the effort.