Bugs in decoder found by fuzzing

[killercup]

Found the following:

• "thread '' panicked at 'No such local time'"
  From: chrono-0.2.25/src/offset/mod.rs:151 via src/decoder/mod.rs:172
• "thread '' panicked at 'attempt to multiply with overflow'" - src/decoder/mod.rs:172
• "thread '' panicked at 'attempt to subtract with overflow'" src/decoder/mod.rs:45
• "AddressSanitizer failed to allocate 0xffffffff93000000 bytes" (whatever that means in real life)

Full logs: https://gist.github.com/killercup/5e8623e0d8b0fe9868b45eb223ef51d8 (See last few lines for inputs used, in bytes or base64)

See rust-fuzz/targets#51 for sources, I ran it with

$ env ASAN_OPTIONS="detect_odr_violation=0 allocator_may_return_null=1" ./run-fuzzer.sh bson read_bson

cc rust-fuzz/targets#39

[zonyitoo]

cc @kyeah

[kyeah]

nice, thanks 🙇

Gonna use this as a tracking issue instead of splitting it out since this seems like a nice concise set.

• "thread panicked at 'No such local time'"
  From: chrono-0.2.25/src/offset/mod.rs:151 via src/decoder/mod.rs:172
• "thread '' panicked at 'attempt to multiply with overflow'" - src/decoder/mod.rs:172
• "thread '' panicked at 'attempt to subtract with overflow'" src/decoder/mod.rs:45
• "AddressSanitizer failed to allocate 0xffffffff93000000 bytes"

[daniellockyer]

Hey @kyeah & @zonyitoo,

It looks like chrono comes with a timestamp_opt method that would allow you to capture the error instead of allowing chrono to panic.

[zonyitoo]

@neosilky Is this what you are expecting? :)

[daniellockyer]

@zonyitoo Yes - nice job!

So I'm still hitting the attempt to subtract with overflow panic in src/decoder/mod:50 and I'm getting two fatal runtime error: memory allocation failed errors, which are weird.

[zonyitoo]

attempt to subtract with overflow

That's weird... src/decoder/mod:50 is read_string, which is a UTF-8 string in BSON. In BSON's spec, UTF-8 String must be ended with \x00, which means that the length of string must be >= 1.

string ::= int32 (byte*) "\x00" String - The int32 is the number bytes in the (byte*) + 1 (for the trailing '\x00'). The (byte*) is zero or more UTF-8 encoded characters.

What should I do for length = 0? ... Hmmm....

Can you dump the original binary data?

[zonyitoo]

It seems that the Go's implementation will also crash in this case: https://github.com/go-mgo/mgo/blob/v2/bson/decode.go#L771-L772 .

The official Python BSON implementation will raise an exception: https://github.com/mongodb/mongo-python-driver/blob/master/bson/__init__.py#L175-L178 .

[zonyitoo]

It won't panic now. But return a DecoderError.

[daniellockyer]

@zonyitoo Nice one, it looks like you've done a sane thing 😀

I'm now getting one weird memory issue and a 'attempt to multiply with overflow' in src/decoder/mod.rs:208 AKA https://github.com/zonyitoo/bson-rs/blob/master/src/decoder/mod.rs#L208

[zonyitoo]

Ahh.. the nsecs overflows u32 ...

[saghm]

@zonyitoo note that mgo is neither an official MongoDB driver nor actively maintained; the official (albeit pre-stable) Go driver is here

[zonyitoo]

@neosilky This commit should fix the issue.

[zonyitoo]

Wait a minute... ((time % 1000) as u32) * 1_000_000 won't overflow u32 ...

999 * 1_000_000 < u32::max_value() aka 4294967296

@neosilky Can you give me a BSON encoded buffer, which can reproduce attempt to multiply with overflow?

[zonyitoo]

Ah my fault.... the time variable is a i64, which means that it may be < 0.

[zonyitoo]

Fixed. And close.
cc @killercup . Sorry for fixing this issue after over half a year.

[daniellockyer]

Thanks!

[kyeah]

Great investigations, thanks @zonyitoo + @neosilky!