Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bugs in decoder found by fuzzing #64

Closed
3 of 4 tasks
killercup opened this issue Mar 14, 2017 · 17 comments
Closed
3 of 4 tasks

Bugs in decoder found by fuzzing #64

killercup opened this issue Mar 14, 2017 · 17 comments

Comments

@killercup
Copy link

killercup commented Mar 14, 2017

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
Copy link
Contributor

cc @kyeah

@kyeah
Copy link
Contributor

kyeah commented Mar 15, 2017

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
Copy link

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
Copy link
Contributor

zonyitoo commented Jun 27, 2018

@neosilky Is this what you are expecting? :)

@daniellockyer
Copy link

@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
Copy link
Contributor

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
Copy link
Contributor

zonyitoo commented Jun 27, 2018

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
Copy link
Contributor

zonyitoo commented Jun 27, 2018

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

@daniellockyer
Copy link

daniellockyer commented Jun 27, 2018

@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
Copy link
Contributor

Ahh.. the nsecs overflows u32 ...

@saghm
Copy link
Contributor

saghm commented Jun 28, 2018

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

@zonyitoo
Copy link
Contributor

@neosilky This commit should fix the issue.

@zonyitoo
Copy link
Contributor

zonyitoo commented Jul 3, 2018

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
Copy link
Contributor

zonyitoo commented Jul 3, 2018

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

zonyitoo added a commit that referenced this issue Jul 3, 2018
@zonyitoo
Copy link
Contributor

zonyitoo commented Jul 3, 2018

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

@zonyitoo zonyitoo closed this as completed Jul 3, 2018
@daniellockyer
Copy link

Thanks!

@kyeah
Copy link
Contributor

kyeah commented Jul 3, 2018

Great investigations, thanks @zonyitoo + @neosilky!

lrlna pushed a commit to lrlna/bson-rs that referenced this issue Feb 27, 2019
lrlna pushed a commit to lrlna/bson-rs that referenced this issue Feb 27, 2019
lrlna pushed a commit to lrlna/bson-rs that referenced this issue Feb 27, 2019
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants