pelletier / go-toml

Go library for the TOML file format
https://github.com/pelletier/go-toml
Other
1.72k stars 209 forks source link

ossfuzz 55562: timeout in fuzz_toml #846

Closed pelletier closed 1 year ago

pelletier commented 1 year ago

https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=55562&q=label%3AProj-go-toml

Reproduction test case: clusterfuzz-testcase-minimized-fuzz_toml-5579932680192000.zip

pelletier commented 1 year ago

I suspect the timeout is specific to oss-fuzz. This input generates ~1.3GB of output. I don't know performance characteristics of the machines running oss-fuzz, but I wouldn't be surprised if they are fairly low. This test case executes in ~8s on a M1 MBP.

manunio commented 1 year ago

I suspect the timeout is specific to oss-fuzz. This input generates ~1.3GB of output. I don't know performance characteristics of the machines running oss-fuzz, but I wouldn't be surprised if they are fairly low. This test case executes in ~8s on a M1 MBP.

Hi, would you be interested in reducing input size for fuzz_targets?

 package toml

 func FuzzToml(data []byte) int {
+       if len(data) >= 10240 {
+               return 0
+       }
+
        var v interface{}
        err := Unmarshal(data, &v)
        if err != nil {

As timeout bugs slow down fuzzing significantly since fuzz target hangs on the processing of those inputs.

pelletier commented 1 year ago

That's a fair point. Thank you for tackling this!

pelletier commented 1 year ago

Would https://github.com/google/oss-fuzz/pull/10116 also close out the monorail tickets that were triggered by larger payloads?

manunio commented 1 year ago

Would https://github.com/google/oss-fuzz/pull/10116 also close out the monorail tickets that were triggered by larger payloads?

No, I have not added that check yet, thought of confirming first with you before doing that, as that check will most likely close Stackoverflow, OOM and Timeout reports.

But before adding a check, I would like to test why the report https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56642 was closed as fixed, as there was no significant change in code base in the following rev range https://github.com/pelletier/go-toml/commit/654811fbc3d827dd62c4685dbb0a5da9a6ee8c4d

pelletier commented 1 year ago

Thanks for the context! Could it be possible that the version of Go used by oss-fuzz changed to newer one, maybe fixing the problem this ticket was hitting before?

manunio commented 1 year ago

Thanks for the context! Could it be possible that the version of Go used by oss-fuzz changed to newer one, maybe fixing the problem this ticket was hitting before?

Hi, go has not been updated recently, its still at 1.19. I think testcase might be flaky. This may need further investigation :)

manunio commented 1 year ago

Hi, This issue along with https://github.com/pelletier/go-toml/issues/845 and https://github.com/pelletier/go-toml/issues/834 can now be closed as they are fixed and closed in oss-fuzz :)

manunio commented 1 year ago

oss-fuzz can also track github issues for you by enabling it using file_github_issue: true at go-toml/project.yaml please note that this will not make report/testcases public at github.

for example: https://github.com/Byron/gitoxide/issues/751

pelletier commented 1 year ago

Good to know! I'll make the change.

manunio commented 1 year ago

Good to know! I'll make the change.

By change, do you mean enabling file_github_issue? I can do that for you :)

pelletier commented 1 year ago

Ah, feel free to merge / get it over the finish line if anything's missing https://github.com/google/oss-fuzz/pull/10317 Thank you!

pelletier commented 1 year ago

@manunio looks like I need to sign some CLA, which seems like it would only work by changing my author email to gmail, which feels like a hassle. Do you mind creating the PR then?

manunio commented 1 year ago

@manunio looks like I need to sign some CLA, which seems like it would only work by changing my author email to gmail, which feels like a hassle. Do you mind creating the PR then?

Sure, will do that :)