Integer overflow in bson_ensure_space (bson.c:613)

Whoopsie bundles a copy of the libbson code which happens to be in other packages as well - mongo-c-driver, php-mongodb, libbson-xs-perl, and duo-unix - so will have to investigate this further to see if it also affects those other packages.

Is this vulnerability not going to be disclosed at the same time as 1830863?
I have done some more investigation into both vulnerabilities and I now believe that this one is the more serious of the two.

This still needs more investigation since it comes from libbson and so affects more packages than just whoopsie (and should probably be disclosed to the upstream libbson project to coordinate how best to resolve it and to determine whether the current version of libbson is affected etc). Kevin, are you comfortable reporting this upstream - https://docs.mongodb.com/manual/tutorial/create-a-vulnerability-report/ as libbson is maintained at https://github.com/mongodb/mongo-c-driver/tree/master/src/libbson as part of the mongo-c-driver project?

Hi Alex,

Do you know which version of libbson you are using? The link that you sent me (https://github.com/mongodb/mongo-c-driver/tree/master/src/libbson) looks like a very different version of libbson than the one which is bundled with the whoopsie source code. The version of libbson that whoopsie is using looks much more like this:

https://github.com/10gen-archive/mongo-c-driver-legacy

That repository doesn't look like it has been maintained since 2013 though. It also isn't completely identical to the code in whoopsie, so maybe it has been maintained somewhere else?

Thanks,

Kev

Ok thanks for the clarification - there appears to be at least one other package with this same code (in Debian, and so likely also in Ubuntu) - https://codesearch.debian.net/search?q=bson_ensure_space - which is duo-unix. Since this does appear unmaintained upstream, we will look at how best to resolve it for whoopsie and can then determine the best course of action for other packages.

CVE-2019-11484 for the integer overflow in bson_ensure_space.

Thanks

Please review the attached debdiff as a suggested fix for this issue.

From debian/changelog:
 * SECURITY UPDATE: Integer overflow when handling large bson objects (LP: #1830865)
   - lib/bson/bson.c, lib/bson/bson.h, src/whoopsie.c: use size_t
     for size instead of int to prevent integer overflows.
   - lib/bson/bson.c: ensure bson objects are not bigger than INT_MAX.
   - CVE-2019-11484
 * src/whoopsie.c: prevent freeing a NULL server response string.

Additional comments:
I have modified some int's used for size even in functions that are not used by whoopsie so we are not hit by this again in case whoopsie changes.
I added a check on size so it does not go over bson int32 specification and I modified a check so it always verifies that the total size is also under max int32 - previously it only did this check 'if( new_size < b->dataSize )'.
Whoopsie would try to free the 's' string even when bsonify failed, thus triggering a NULL assert, so I moved the string declaration and the free call inside the scope of the crash file upload.

New upload with the right file extension, same contents.

Thanks for the patch Tiago - looks good - note, the conversion to size_t still might allow integer overflow BUT since this is unsigned it shouldn't allow access to memory via negative indices, AND with your extra checks against INT_MAX this should be quite safe now.

Hi Tiago,

Thank you very much for the patch. I am very sorry for not reviewing it sooner. I have a few suggestions:

1. This block of code, which you retained from the original, doesn't make a lot of sense to me:

     if( ( b->dataSize + bytesNeeded ) < INT_MAX )
         new_size = INT_MAX;
     else {
         b->err = BSON_SIZE_OVERFLOW;
         return BSON_ERROR;
     }

   That's going to lead to a 2GB allocation almost every time, which is a bit wasteful.
   I think that entire block of code serves no useful purpose and you should just delete it.

2. Just in case the code is ever compiled for a 32-bit machine, I would recommend adding some bounds checking around the multiplication of new_size by 1.5 (bson.c, line 621).

3. In bson_append_estart, change the type of local variable "len" to size_t.

3. In bson_append_string_base, change the type of parameter "len" to size_t.

4. In bson_append_string_base, change the type of local variable "sl" to size_t.

5. In encoding.c, there are number of parameters and local variables of type int. They should all be changed to size_t.

Thanks,

Kev

FYI, I have a working exploit for this bug now. An unprivileged user can get a shell as the whoopsie user. I had to chain this bug with Apport bug 1839795 to obtain ASLR offsets for whoopsie. Here's the video:

https://drive.google.com/file/d/1K7KYw8umXBXLRpiwXso6Va4VZjkBNu-J/view?usp=sharing

Kevin, thank you very much for your thoughtful review and proposed fixes.

Please review the attached debdiff with a suggested fix for this security issue. I have added a stronger check on the bounds of a few operations done in bson_ensure_space, hopefully that is enough to cover all the cases where an overflow could happen (including 32-bit) - there's also the fact that bson is limited to a signed int by design, thus the checks had to be done against INT_MAX instead of UINT_MAX.

Best regards

Hi Tiago,

Thanks! The new patch looks good to me.

Kev

Tiago - thanks for the updated patch - looks good to me as well.

Tiago - one more thing - can you please propose a coordinated release date for this update so we can coordinate our release of the update with Kev's public notification of the vulnerability?

Ok a CRD has been confirmed for this issue - Tuesday next week - 2019/10/29

Thanks for letting me know.

Kev

Patch for eoan.

Patch for disco.

Patch for bionic.

Patch for xenial.

Patch for trusty - I haven't tested it for this particular release.

Thanks Tiago - I have reviewed and verified the patches for each release - the ESM folks will handle the trusty update so I will leave it to them to test that.

This bug was fixed in the package whoopsie - 0.2.66ubuntu0.1

---------------
whoopsie (0.2.66ubuntu0.1) eoan-security; urgency=medium

  * SECURITY UPDATE: Integer overflow when handling large bson
    objects (LP: #1830865)
    - lib/bson/bson.c, lib/bson/bson.h, src/whoopsie.c: use size_t
      for size instead of int to prevent integer overflows.
    - lib/bson/bson.c: ensure bson objects are not bigger than INT_MAX.
    - CVE-2019-11484
  * src/whoopsie.c: prevent freeing a NULL server response string.

 -- Tiago Stürmer Daitx <email address hidden> Wed, 04 Sep 2019 01:33:49 +0000

This bug was fixed in the package whoopsie - 0.2.52.5ubuntu0.2

---------------
whoopsie (0.2.52.5ubuntu0.2) xenial-security; urgency=high

  * SECURITY UPDATE: Integer overflow when handling large bson
    objects (LP: #1830865)
    - lib/bson/bson.c, lib/bson/bson.h, src/whoopsie.c: use size_t
      for size instead of int to prevent integer overflows.
    - lib/bson/bson.c: ensure bson objects are not bigger than INT_MAX.
    - CVE-2019-11484

 -- Tiago Stürmer Daitx <email address hidden> Mon, 14 Oct 2019 14:17:30 +0000