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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants