add four new fuzzing harnesses#4076
Conversation
The main goal here is to increase code coverage of the OSS-Fuzz project. A recent code coverage report is available here: https://storage.googleapis.com/oss-fuzz-coverage/pacemaker/reports/20260331/linux/src/report.html Signed-off-by: David Korczynski <david@adalogics.com>
|
Can one of the project admins check and authorise this run please: https://haci.fast.eng.rdu2.dc.redhat.com/job/pacemaker/job/pacemaker-pipeline/job/PR-4076/1/input |
| xmlNode *xml = NULL; | ||
| xmlNode *result = NULL; | ||
|
|
||
| if (size < 20) { |
There was a problem hiding this comment.
Can we get comments explaining the magic size numbers, here and in the other harnesses?
| // Run the ACL filtered copy with a non-root user | ||
| // pcmk_acl_required() returns false for "root" and "hacluster", so we use | ||
| // a regular user name to ensure ACL processing is actually exercised. | ||
| xml_acl_filtered_copy("fuzzuser", xml, xml, &result); |
There was a problem hiding this comment.
Would it be worth fuzzing with different usernames (returning -1 for "root" or "hacluster" to exclude those from the corpus)? I'm new to libfuzzer and unsure of best practices for coverage.
Speaking of which... I don't know whether or how the corpus gets seeded with valid inputs. If we're relying on purely random data here, I would expect it to take an unreasonably long time to get anything remotely meaningful, even if we get valid XML. If all we care about is testing garbage input, that's fine.
| // a regular user name to ensure ACL processing is actually exercised. | ||
| xml_acl_filtered_copy("fuzzuser", xml, xml, &result); | ||
|
|
||
| if (result != NULL) { |
There was a problem hiding this comment.
This doesn't need to be guarded. pcmk__xml_free() is NULL-safe.
| * This provides enough structure for XPath operations to have meaningful | ||
| * targets (nodes, resources, constraints, status). | ||
| */ | ||
| static const char *BASE_CIB = |
There was a problem hiding this comment.
We usually reserve all-caps for #define'd constants. I'd prefer to see this as either a #define or base_cib
| * targets (nodes, resources, constraints, status). | ||
| */ | ||
| static const char *BASE_CIB = | ||
| "<cib admin_epoch=\"1\" epoch=\"1\" num_updates=\"0\">" |
There was a problem hiding this comment.
I'm pretty sure you can use single quotes within the XML and avoid all the backslash-escaping
| @@ -0,0 +1,55 @@ | |||
| /* | |||
There was a problem hiding this comment.
We should either change the name of this file or change our documentation.
That directory has a file for each fuzzed source file, named the same except ending in
_fuzzer.c(for example,lib/common/fuzzers/strings_fuzzer.chas fuzzing forlib/common/strings.c).
There is no xml_parse.c file.
| xml = pcmk__xml_parse(input); | ||
|
|
||
| // If parsing succeeded, exercise some read-only operations on the result | ||
| if (xml != NULL) { |
There was a problem hiding this comment.
We might as well be consistent with the other three files in this commit, and return early if xml == NULL, de-nesting the body of this if statement.
| pcmk__xe_id(xml); | ||
|
|
||
| // Iterate children — exercises XML tree traversal | ||
| for (xmlNode *child = pcmk__xe_first_child(xml, NULL, NULL, NULL); |
There was a problem hiding this comment.
can use const xmlNode *child
|
|
||
| // If parsing succeeded, exercise some read-only operations on the result | ||
| if (xml != NULL) { | ||
| // Access the element name and ID (common post-parse operations) |
There was a problem hiding this comment.
Where are we accessing the element name?
| // Parse the fuzz input as XML | ||
| xml = pcmk__xml_parse(input); | ||
| if (xml == NULL) { | ||
| free(input); |
There was a problem hiding this comment.
If you want, you can do all the freeing after a done: label and just goto done here. We frequently do cleanup using goto done, even though we don't use goto for much of anything else. It helps us not to forget to free anything in early returns.
It doesn't matter to me. Note that this applies to other files in this commit too.
2 participants
The main goal here is to increase code coverage of the OSS-Fuzz project. A recent code coverage report is available here:
https://storage.googleapis.com/oss-fuzz-coverage/pacemaker/reports/20260331/linux/src/report.html