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

zip, asn1, json: 4 panics found by native go-fuzz and fuzzit #34715

Open
yevgenypats opened this issue Oct 6, 2019 · 9 comments
Open

zip, asn1, json: 4 panics found by native go-fuzz and fuzzit #34715

yevgenypats opened this issue Oct 6, 2019 · 9 comments

Comments

@yevgenypats
Copy link

@yevgenypats yevgenypats commented Oct 6, 2019

Hey Team,

I run go-fuzz native engine (not the libfuzzer integration) on Fuzzit.dev with @dvyukov fuzz targets and found the following 3 panics:

I tried creating a playground environment for this but it doesn't propagate crashes https://play.golang.org/p/jLuEPLSCORI

first hex crash for github.com/dvyukov/go-fuzz-corpus/zip

"504b01023030303030303030303030303030303030303030303030300800010001003030303030303030303030303030303030504b030430504b0102303030303030303030303030303030303030303030303030020001000000303030303030303030303030303030504b0102303030303030000030300700000000003030303000000000000008000100303030303030303033000000504b050630303030303003003030303000000000010030"
backtrace:

hdr0: &zip.FileHeader{Name:"", Comment:"0", NonUTF8:false, CreatorVersion:0x3014, ReaderVersion:0x14, Flags:0x3038, Method:0x0, Modified:time.Time{wall:0x0, ext:62448991292, loc:(*time.Location)(nil)}, ModifiedTime:0x3030, ModifiedDate:0xff87, CRC32:0x0, CompressedSize:0x0, UncompressedSize:0x0, CompressedSize64:0x0, UncompressedSize64:0x0, Extra:[]uint8{0x50, 0x4b, 0x5, 0x6, 0x30, 0x30, 0x30, 0x30, 0x55, 0x54, 0x5, 0x0, 0x1, 0x3c, 0x5, 0xae, 0x12}, ExternalAttrs:0x30303030}
hdr1: &zip.FileHeader{Name:"", Comment:"0", NonUTF8:false, CreatorVersion:0x3014, ReaderVersion:0x14, Flags:0x3038, Method:0x0, Modified:time.Time{wall:0x0, ext:66488277692, loc:(*time.Location)(nil)}, ModifiedTime:0x3030, ModifiedDate:0xff87, CRC32:0x0, CompressedSize:0x0, UncompressedSize:0x0, CompressedSize64:0x0, UncompressedSize64:0x0, Extra:[]uint8{0x50, 0x4b, 0x5, 0x6, 0x30, 0x30, 0x30, 0x30, 0x55, 0x54, 0x5, 0x0, 0x1, 0x3c, 0x5, 0xae, 0x12}, ExternalAttrs:0x30303030}
panic: corrupted header

goroutine 1 [running]:
main.Fuzz(0xc0000d2000, 0xae, 0x2ae, 0xae)
        /Users/yevgenyp/go/src/github.com/dvyukov/go-fuzz-corpus/zip/main.go:113 +0x101f
main.main()
        /Users/yevgenyp/go/src/github.com/dvyukov/go-fuzz-corpus/zip/main.go:127 +0xc1
exit status 2

second hex crash for github.com/dvyukov/go-fuzz-corpus/zip

"504b01023030303030303030303030303030303030303030303030300800010001003030303030303030303030303030303030504b030430504b0102303030303030303030303030303030303030002000303030020001000000303030303030303030303030303030504b01023030303030300000303030307924d2d0303030300d000000080001000100303030303030303033000000504b05063030302f30300300ffff000000000000010042"

backtrace:

panic: zip: write to directory

goroutine 1 [running]:
main.Fuzz(0xc0000c2000, 0xae, 0x2ae, 0xae)
        /Users/yevgenyp/go/src/github.com/dvyukov/go-fuzz-corpus/zip/main.go:61 +0x1135
main.main()
        /Users/yevgenyp/go/src/github.com/dvyukov/go-fuzz-corpus/zip/main.go:127 +0xc1
exit status 2

second hex crash for github.com/dvyukov/go-fuzz-corpus/asn1

"1b0430b43030"

backtrace:

panic: asn1: string not valid UTF-8

goroutine 1 [running]:
main.Fuzz(0xc0000a2000, 0x6, 0x206, 0x6)
        /Users/yevgenyp/go/src/github.com/dvyukov/go-fuzz-corpus/asn1/asn1.go:48 +0x87d
main.main()
        /Users/yevgenyp/go/src/github.com/dvyukov/go-fuzz-corpus/asn1/asn1.go:110 +0xc1
exit status 2

second hex crash for github.com/dvyukov/go-fuzz-corpus/json

repro link https://play.golang.org/p/DS-JC5JhbKG

@thepudds
Copy link

@thepudds thepudds commented Oct 6, 2019

@yevgenypats, very nice to see you have picked up native dvyukov/go-fuzz support.

Regarding the playground error you were seeing, I suspect that the Error communicating with remote server error reported by your playground link might be due to timing out (or maybe some other error) when trying to download the fairly large github.com/dvyukov/go-fuzz-corpus dependency. (Being able to download third-party dependencies on the playground is still a fairly new feature, and it is not quite perfect yet, including around error messages if something goes wrong).

I tweaked your playground example to remove the direct import of github.com/dvyukov/go-fuzz-corpus, and it now successfully runs and reports the expected panic: zip: write to directory:

https://play.golang.org/p/h67gkTQnz8R

I changed the go-fuzz-corpus fuzz.DeepEqual to instead use reflect.DeepEqual. That is not always a valid transformation, e.g., from the doc:

// DeepEqual is reflect.DeepEqual except that:
// 1. nil and empty slice/string are considered equal
// 2. NaNs compare equal.

... but it should be OK here, I think. I also threw in a hex.DecodeString.

In any event, at least for now, playground links are a good way to share the reproducer, especially for now as people hopefully start to get more accustom to more fuzzing results for the stdlib.

(I saw these panics found by fuzzitbot via a tweet from @dvyukov, so people are probably starting to see this already, but I'll throw in a CC to @josharian and @FiloSottile in case interested.)

@thepudds
Copy link

@thepudds thepudds commented Oct 6, 2019

@yevgenypats for the asn1 panic, what exact version of dvyukov/go-fuzz-corpus/asn1/asn1.go are you using, or alternatively, could you post a playground link?

The line numbers in the reported panic don't seem to line up with the latest code in:

https://github.com/dvyukov/go-fuzz-corpus/blob/master/asn1/asn1.go

(But I also might be confused, or perhaps my Sunday AM coffee has not kicked in yet for me...).

@thepudds
Copy link

@thepudds thepudds commented Oct 6, 2019

From a very quick triage (or in other words, don't really trust this, and others should look more carefully.... Also, I'm not quite sure the line numbers line up, so I might be looking at the wrong code):

  1. The first reported panic for github.com/dvyukov/go-fuzz-corpus/zip might be due to a failed round-trip validation with a mismatched header after compress/decompress.

  2. The second reported panic for github.com/dvyukov/go-fuzz-corpus/zip might be due to a "user error" of attempting to write to a directory. There might be a need to guard against reporting that in the fuzz function. Alternatively, maybe that is an actual stdlib bug.

  3. For the reported github.com/dvyukov/go-fuzz-corpus/asn1 panic, I can't quite see where it is panicking (e.g., maybe after an error was returned from asn1.Unmarshal, or maybe after an error was returned from asn1.Marshal, or somewhere else).

@thepudds
Copy link

@thepudds thepudds commented Oct 6, 2019

For the 1st zip panic (panic: corrupted header), I created this playground link:

https://play.golang.org/p/Ajt4bobuTwL

(That copies in the go-fuzz-corpus DeepEqual function, in contrast to the playground link above for the 2nd zip panic, which used reflect.DeepEqual directly).

@yevgenypats
Copy link
Author

@yevgenypats yevgenypats commented Oct 7, 2019

Hey @thepudds thanks for the analysis!:)

using your instructions I've created another playground link for another crash (I'm not sure yet if it's a real problem or more of a problem in the fuzz functions itself though).

https://play.golang.org/p/DS-JC5JhbKG

@yevgenypats yevgenypats changed the title FuzzitBot: 3 panics in zip and asn1 found by native go-fuzz and fuzzit FuzzitBot: 4 panics in zip,asn1,json found by native go-fuzz and fuzzit Oct 7, 2019
@thepudds
Copy link

@thepudds thepudds commented Oct 7, 2019

@yevgenypats For the json panic you just posted at https://play.golang.org/p/DS-JC5JhbKG (panic number 4 overall here), I think that is due to duplicate keys not surviving the round-trip checks in the fuzz function, which is dvyukov/go-fuzz-corpus#3. That's expected behavior for the json package, e.g., see comment at #24415 (comment).

If that is correct, then that would be something to fix within the fuzz function.

Would you be able to post a playground link for the asn1 panic reported here?

@yevgenypats
Copy link
Author

@yevgenypats yevgenypats commented Oct 7, 2019

@andybons andybons changed the title FuzzitBot: 4 panics in zip,asn1,json found by native go-fuzz and fuzzit zip, asn1, json: 4 panics found by native go-fuzz and fuzzit Oct 7, 2019
@andybons andybons added this to the Unplanned milestone Oct 7, 2019
@andybons
Copy link
Member

@andybons andybons commented Oct 7, 2019

Once you have confirmed repro steps for each package, would you mind filing separate issues for them (unless they have the same underlying cause)? Thanks.

@thepudds
Copy link

@thepudds thepudds commented Oct 7, 2019

Here is a playground link that I think corresponds to asn1 failure reported above: https://play.golang.org/p/6DqPzc84117

It looks like there is an asn1.Unmarshal that returns without error, and then the result is used in asn1.Marshal, which returns the asn1: string not valid UTF-8 error (which the Fuzz function sees and then calls panic).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.