fuzzing: cifuzz workflow, fix oss-fuzz issues, updated dir name

[pkillarjun]

Continuing the fuzzing integration: #1291.

Adding cifuzz for checking build failures and PR fuzzing.
Using the main example provided by the OSS-Fuzz team, with an updated time of only 300 seconds.

[pkillarjun]

FAQ

Upload Crash

Upload Crash uses artifacts to save crashes found by fuzzing as artifacts.

Artifacts: When the fuzzer crashes the input file that causes the crash is uploaded as an artifact.
To download the artifact, do the following steps:

Upload Sarif

There is no proper information available that I could find.
But, when a crash is found, the OSS-Fuzz infrastructure is alerted using SARIF files. A nice detailed report is then generated for maintainers with a better UI.

My Souce: Souce-1, Souce-2 .

[github-advanced-security]

This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation.

[pkillarjun]

I doubt that.

2024-06-18 02:24:35,946 - root - INFO - Fuzzer: fuzz_http_h1p_peer. Detected bug.
2024-06-18 02:24:35,946 - root - INFO - Trying to reproduce crash using: /tmp/tmp66kfxbhm/crash-293e3405de4ccb599b6e4f770ddf25362554727c.

Let's wait for 24 hours and then see.

Edit:
Hmmm, indeed, it was a sync problem.

[pkillarjun]

Fix some harness bugs 3b99bf6

• https://oss-fuzz.com/testcase-detail/4805770206576640
• https://oss-fuzz.com/testcase-detail/5276662569172992
• https://oss-fuzz.com/testcase-detail/4581551606661120

[ac000]

This

conf: add %Z to resolve oss-fuzz issue 69758 use-of-uninitialized-value

Signed-off-by: Arjun <[email protected]>

Could do with a commit message, I wasn't even aware (or if I was I had completely forgotten) that nxt_sprintf() doesn't nul-terminate strings by default.

But then at the very least it probably also needs a

Co-developed-by: Zhidao HONG <[email protected]>
Signed-off-by: Zhidao HONG <[email protected]>

@hongzhidao are you OK with that?

The commit subject prefix could probably be tstr, conf:. Perhaps something like

tstr, conf: Ensure error strings are nul-terminated

Are these bug reports publicly accessible?

If not I don't think we should mention specific bug numbers but just a general 'found with oss-fuss`, if so, it would be better to reference it with a link

Link: <https://oss-fuzz.com/testcase-detail/5545917827055616>

I think it would actually be better to open a separate pull-request for this where we can discuss the finer details and to get this fix in...

[hongzhidao]

I'm fine with it.
It can be a separate fix.

I wasn't even aware (or if I was I had completely forgotten) that nxt_sprintf() doesn't nul-terminate strings by default.

Yep, it's not nul-temmiate by default.

The commit subject prefix could probably be tstr, conf:. Perhaps something like
tstr, conf: Ensure error strings are nul-terminated

We could show the concrete function nxt_tstr_test() which is not used there.

Btw, I'd like to use pkillarjun as the signed-off instead of me. He reported the issue and fixed it with my suggestion.

[ac000]

I'm fine with it. It can be a separate fix.

It it's in a separate PR then it's not reliant on getting this whole thing in...

I wasn't even aware (or if I was I had completely forgotten) that nxt_sprintf() doesn't nul-terminate strings by default.

Yep, it's not nul-temmiate by default.

The commit subject prefix could probably be tstr, conf:. Perhaps something like
tstr, conf: Ensure error strings are nul-terminated

We could show the concrete function nxt_tstr_test() which is not used there.

Yes, that would be part of the commit message explaining that you need to explicitly nul-terminate the string.

Btw, I'd like to use pkillarjun as the signed-off instead of me. He reported the issue and fixed it with my suggestion.

You can have multiple Signed-off-by:'s and a Co-developed-by: should be followed by a Signed-off-by:

So the whole thing might look like

tstr, conf: Ensure error strings are nul-terminated

This issue was found with oss-fuzz.                                             
                                                                                
  ==18420==WARNING: MemorySanitizer: use-of-uninitialized-value                 
              #0 0x55dd798a5797 in nxt_vsprintf unit/src/nxt_sprintf.c:163:31   
              #1 0x55dd798d5bdb in nxt_conf_vldt_error unit/src/nxt_conf_validation.c:1525:11
              #2 0x55dd798dd4cd in nxt_conf_vldt_var unit/src/nxt_conf_validation.c:1560:16
              #3 0x55dd798dd4cd in nxt_conf_vldt_if unit/src/nxt_conf_validation.c:1592:16
              #4 0x55dd798d55f4 in nxt_conf_vldt_object unit/src/nxt_conf_validation.c:2815:23
              #5 0x55dd798d6f84 in nxt_conf_vldt_access_log unit/src/nxt_conf_validation.c:3426:11
              #6 0x55dd798d55f4 in nxt_conf_vldt_object unit/src/nxt_conf_validation.c:2815:23
              #7 0x55dd798d47bd in nxt_conf_validate unit/src/nxt_conf_validation.c:1421:11
              #8 0x55dd79871c82 in LLVMFuzzerTestOneInput unit/fuzzing/nxt_json_fuzz.c:67:5
              #9 0x55dd79770620 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:614:13
              #10 0x55dd7975adb4 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:327:6
              #11 0x55dd7976084a in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:862:9
              #12 0x55dd7978cc42 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
              #13 0x7e8192213082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/libc-start.c:308:16
              #14 0x55dd7975188d in _start                                      
                                                                                
            Uninitialized value was created by an allocation of 'error.i' in the stack frame
              #0 0x55dd798dd42b in nxt_conf_vldt_var unit/src/nxt_conf_validation.c:1557:5
              #1 0x55dd798dd42b in nxt_conf_vldt_if unit/src/nxt_conf_validation.c:1592:16
                                                                                
The issue was in nxt_tstr_test() where we create an error message with
nxt_sprintf(), where this error message is then later used with the
'%s' format specifier which expects a nul-terminated string, but by
default nxt_sprintf() doesn't nul-terminate, you must use the '%Z'
specifier to signify a '\0' at the end of the string.            
                                                                                
Link: <https://github.com/google/oss-fuzz>
Link: <https://oss-fuzz.com/testcase-detail/5545917827055616>
Co-developed-by: Zhidao HONG <[email protected]>
Signed-off-by: Zhidao HONG <[email protected]>
Reviewed-by: Andrew Clayton <[email protected]>
Signed-off-by: Arjun <[email protected]>
[ Commit message/subject - Andrew ]
Signed-off-by: Andrew Clayton <[email protected]>

The second link may or may not be included, depending if it's publicly accessible or not...

[ac000]

@hongzhidao @pkillarjun

If you're good with the commit message etc, then I'm happy to merge this patch in and then it can simply be dropped from this PR...

EDIT: Nuts, only I can't because it would need approving... I'll create a new PR for this patch...

[ac000]

PR for bug fix #1342

[ac000]

OK @pkillarjun you can remove this patch from this PR now, thanks!

conf: add %Z to resolve oss-fuzz issue 69758 use-of-uninitialized-value

[pkillarjun]

This can be merge now.

[ac000]

Hi, regarding

fuzzing: fix Null-dereference 69754, 69745, 69741 

I assume those numbers are some bug ticket references?

As a rule we don't put those kinds of things in the commit subject. If those things are publicly accessible then you can reference them via Link: tags...

However...

This is just in the fuzzing code, not in Unit right?

So is this fixing fuzzing code that was previously committed?

If so it could do with a Fixes: tag, e.g

Fixes: SHORT_HASH ("COMMIT_SUBJECT")

E.g

Fixes: c1e3f02f9 ("Compile with -fno-strict-overflow")

[ac000]

Nuts!

fuzzing: updated build-fuzz.sh help echo dir

This could do with a

Fixes: 965fc94e4 ("fuzzing: add fuzzing infrastructure in build system")

I also notice the ./fuzzing/README.md still has a bunch of references to src/fuzz/

[ac000]

Hi @pkillarjun

The Fixes: tag is like any other tag and goes at the end of the commit message, not in the subject!

If those commits are fixing previous commits, please explain in the commit message what they are fixing.

[pkillarjun]

Hi @pkillarjun

The Fixes: tag is like any other tag and goes at the end of the commit message, not in the subject!

If those commits are fixing previous commits, please explain in the commit message what they are fixing.

Alright, thanks.
Ah, can you rereviw this now, i think these new comment will do the job.

[ac000]

Thanks Arjun!