Fix AFL++ fuzzing harness input handling & add security workflows

[Easton97-Jens]

what

• Fixes incorrect handling of AFL++ input data in the fuzzing harness

• Correctly uses the input buffer (buf) to construct the string

• Adds guard for invalid/empty input (read_bytes <= 0)

• Prevents potential null pointer dereference in Operator::instantiate()

• Adds fallback definition for __AFL_LOOP (enables non-AFL builds)

• Adds additional security workflows:

  • CodeQL analysis
  • Runtime sanitizers (ASan/UBSan)
  • AFL++ fuzzing smoke test

why

• Previous implementation did not properly use fuzzer input
  → resulting in ineffective fuzzing (no real coverage)
• Proper input handling is required to test transformations and operators with real data
• Null check prevents crashes outside AFL environments
• New workflows improve security, stability, and detection of runtime issues (memory, UB, DoS)
• Goal: Detect bugs and vulnerabilities early

references

• AFL++ documentation (input handling best practices)
• ModSecurity fuzzing harness (test/fuzzer/afl_fuzzer.cc)

[github-advanced-security]

You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool.

What Enabling Code Scanning Means:

• The 'Security' tab will display more code scanning analysis results (e.g., for the default branch).
• Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results.
• You will be able to see the analysis results for the pull request's branch on this overview once the scans have completed and the checks have passed.

For more information about GitHub Code Scanning, check out the documentation.

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[sonarqubecloud]

Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

[shotintoeternity]

Independently hit this same bug from the other direction — was preparing a patch
for afl_fuzzer.cc:145 before finding this PR. Confirming the core fix is
correct, and adding some analysis that might help it get reviewed.

Why the original line is wrong. std::string(read_bytes, 128) selects the
fill constructor basic_string(size_type count, CharT ch) — read_bytes copies
of the character 128. Overload resolution isn't ambiguous, which is likely why
it survived so long: the pointer-and-length constructor isn't viable because
ssize_t doesn't convert to const char*, and the iterator-pair constructor
isn't viable because deduction conflicts (ssize_t vs int). The fill
constructor is the only candidate. buf appears exactly three times in the file
— declaration, memset, read — and is never read from.

Consequence. The harness's input domain is a run of 0x80 bytes whose only
attacker-controlled property is its length, 0-128 — 129 distinct inputs total.
AFL mutations of file contents have no effect on behaviour; only mutations
changing file size do. So t:hexDecode never sees a hex digit, Base64Decode
never sees base64, DetectSQLi/DetectXSS never see a quote or angle bracket.
Worth emphasising because it isn't a silent no-op: the harness still executes
~36 transformations and ~25 operators per iteration, so it produces coverage
edges and looks like it's working. It just produces the same edges forever.

Verified locally on the two constructs side by side (Apple clang 17, -std=c++17):

OLD  len=12  bytes=80 80 80 80 80 80 80 80 80 80 80 80
NEW  len=12  bytes=48 45 3c 73 63 72 69 70 74 3e 27 41   -> "HE<script>'A"

Provenance. git log -L145,145:test/fuzzer/afl_fuzzer.cc returns one commit:
c2d9a153 ("Adds support to afl fuzzer in the build system", 2015-12-22). The
line is unchanged since the harness was written. 23 later commits touched
test/fuzzer/, three of which read this exact region — b8caf6e3 ("Make 's'
const reference to avoid the copy", which edits line 146 directly below),
9b40a045 (cppcheck cosmetics) and c4339107 (static-analysis fixes). The
compiler has been flagging it the whole time:

warning: implicit conversion from 'int' to 'char' changes value
         from 128 to -128 [-Wconstant-conversion]

It went unseen because the harness is gated behind --enable-afl-fuzz
(off by default, configure.ac:284), so no CI job surfaced the warning.

One suggestion, offered as a possible reason this has sat for three months.
The harness fix here is three lines and is correct on its own merits. It's
bundled with three new CI workflows, an op_test signature change, and some
comment escaping — 422 additions total. The ASan/UBSan Linux check currently
failing is a job defined by this PR's own runtime-sanitizers.yml, and none of
the three workflow files exist on master yet, so the thing blocking the fix is
infrastructure the same PR introduces.

Splitting the afl_fuzzer.cc input-handling fix into its own small PR would let
it be reviewed and merged on its own, with the CI/workflow additions tracked
separately where they can be discussed on their own merits. Happy to open that
split PR if it would help, or to leave it entirely to @Easton97-Jens — it's
their find and their patch, I'm only confirming it.

Not a security issue, for the record, so nobody needs to escalate: the
harness is a build-gated test utility, off by default, and none of this is
attacker-reachable. It's a correctness bug in tooling — the practical impact is
that the transformation and operator surface has effectively never been fuzzed,
which matters mainly for what it implies about coverage rather than for any
live exposure.

[Easton97-Jens]

Thank you for the detailed analysis and for independently confirming the bug.

I agree that splitting the focused afl_fuzzer.cc fix from the broader CI and workflow changes would be the best way forward.

Unfortunately, I do not currently have enough time to prepare and maintain the separate PR, as I am working on several ModSecurity connector projects. If you are willing to open the focused PR, please feel free to do so — I would greatly appreciate your help.

You are welcome to reuse or adapt the relevant part of my patch and reference this PR for the background and analysis. I am completely fine with you taking care of getting this specific fix reviewed and merged.

Thank you again for offering to help.

[shotintoeternity]

Correcting something I got wrong above.

I wrote that the harness "still executes ~36 transformations and ~25 operators per
iteration, so it produces coverage edges and looks like it's working." After some
analysis, I found out that was not correct, and it doesn't build at all.

On current v3/master:

$ c++ -fsyntax-only -std=c++17 -D'__AFL_LOOP(x)=1' \
      -I. -Iheaders -Isrc -Iothers test/fuzzer/afl_fuzzer.cc
38 errors, all "no matching member function for call to 'evaluate'"

37 of those are the generated transformation lines and 1 is op_test. I defined
__AFL_LOOP on the command line so it is not counted, since afl-clang-fast
supplies it.

Two upstream changes caused this, neither of which touched the harness:

• 5d398907 (merged 2024-08-27) renamed Transformation::evaluate to
  transform(std::string &value, const Transaction *trans) and removed
  Action::evaluate(const std::string&, Transaction*). This broke all 37
  transformation lines.
• 4df297b5 (merged 2024-10-07) changed the fourth parameter of
  Operator::evaluate from std::shared_ptr<RuleMessage> to RuleMessage&, so
  nullptr no longer binds. This broke op_test.

I checked out 5d398907^ and ran the same syntax check there: 0 errors, and one
warning, the -Wconstant-conversion at line 145.

So the harness last compiled in August 2024, which makes the string constructor the
older of the two problems rather than the active one. The conclusion gets stronger
rather than weaker. The transformation and operator surface has not been fuzzed
since August 2024, and for the years before that it was fuzzed against 129 distinct
inputs.

@Easton97-Jens, thank you for the offer. I have opened #3607 with the focused fix.
Your input-handling fix and your op_test signature fix are both in it, and the
find and the analysis are credited to you there.

One implementation note, since it is the single place I diverged from your patch and
it is easy to miss on review. I construct the transformation classes directly rather
than going through Transformation::instantiate. That function matches with
a.compare(2, ...), which expects the rule-language spelling with the t: prefix,
so class-cased names such as "Base64Decode" fall through to the base
Transformation, whose transform() returns without touching the value. Direct
construction avoids that and keeps the name list compile-checked. Measurements and
the reasoning are in the PR description.