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 DOM Configurator fuzzer #460

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

AdamKorcz
Copy link
Contributor

Adds a fuzzer that targets DOMConfigurator::configure().

char filename[9] = "conf.xml";

FILE *fp = fopen(filename, "wb");
if (!fp) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to provide/generate conf.xml? Otherwise this is always going to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fopen() creates the file if it doesn't exist, if that is what you are concerned of?

Copy link
Contributor

Choose a reason for hiding this comment

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

Testing DOMConfigurator::configure() with an empty file should be just a unit test.

A fuzz test of xml content would be a better use of oss-fuzz resources.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's writing the file a few lines below, so I believe this is properly writing out the data to a file and then having the DOMConfigurator parse the file that was just written.

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you expect this fuzz test to achieve?

Have you considered mimicing the fuzz test in libexpat?

Copy link
Contributor

Choose a reason for hiding this comment

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

For OSS-fuzz to deeply investigate the DOMConfigurator code, it would need to generate mostly valid log4cxx configuration which would be extremely unlikely, as admitted by this concluding comment in this blog post

In practical terms, this means that afl-fuzz won't have as much luck "inventing" PNG files or non-trivial HTML documents from scratch

Copy link
Contributor Author

@AdamKorcz AdamKorcz Jan 27, 2025

Choose a reason for hiding this comment

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

For OSS-fuzz to deeply investigate the DOMConfigurator code, it would need to generate mostly valid log4cxx configuration which would be extremely unlikely, as admitted by this concluding comment in this blog post

In practical terms, this means that afl-fuzz won't have as much luck "inventing" PNG files or non-trivial HTML documents from scratch

libFuzzer will be able to explore the code because of its coverage-guided features. We can help it too by adding a seed and a dictionary which will greatly improve the fuzzers reach from the first iterations. Once the fuzzer is merged, I can add that to the OSS-Fuzz build files.

What do you expect this fuzz test to achieve?

The fuzzer tests the XML parsing of the DOM Configurator.. The DOM configurator has 1000+ LoC which would be great to have fuzz coverage of. I don't see a reason to not test it.

Have you considered mimicing the fuzz test in libexpat?

No. Could you elaborate on the advantages of that?

Testing DOMConfigurator::configure() with an empty file should be just a unit test.

The file will not be empty. The fuzzer writes the test case to the file and then invokes the DOM Configurator so that it parses it.

Signed-off-by: Adam Korczynski <[email protected]>
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

Successfully merging this pull request may close these issues.

3 participants