Integer overflow in bson_ensure_space (bson.c:613)

Bug #1830865 reported by kev
262
This bug affects 1 person
Affects Status Importance Assigned to Milestone
whoopsie (Ubuntu)
High
Unassigned

Bug Description

Dear Ubuntu Security Team,

I would like to report an integer overflow vulnerability in whoopsie. In combination with issue 1830858, this vulnerability may enable an local attacker to read arbitrary files on the system.

I have attached a proof-of-concept which triggers the vulnerability. I have tested it on an up-to-date Ubuntu 18.04. Run it as follows:

bunzip2 PoC.tar.bz2
tar -xf PoC.tar
cd PoC
make
./killwhoopsie2

The PoC works by creating a file named `/var/crash/killwhoopsie.crash`, just over 2GB in size. It then creates a file named `/var/crash/killwhoopsie.upload`, which prompts whoopsie to start processing the .crash file.

This is the source location of the integer overflow bug:

http://bazaar.launchpad.net/~daisy-pluckers/whoopsie/trunk/view/698/lib/bson/bson.c#L613

The problem is that the types of pos, bytesNeeded, and b->dataSize are all int. My PoC triggers an integer overflow in the calculation of pos + bytesNeeded, which causes bson_ensure_space to return immediately on line 614 without allocating more space. This leads subsequently to a heap buffer overflow on line 738:

http://bazaar.launchpad.net/~daisy-pluckers/whoopsie/trunk/view/698/lib/bson/bson.c#L738

Please let me know when you have fixed the vulnerability, so that I can coordinate my disclosure with yours. For reference, here is a link to Semmle's vulnerability disclosure policy: https://lgtm.com/security#disclosure_policy

Thank you,

Kevin Backhouse

Semmle Security Research Team

CVE References

Revision history for this message
kev (kbackhouse2000) wrote :
Revision history for this message
Alex Murray (alexmurray) wrote :

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.

Revision history for this message
kev (kbackhouse2000) wrote :

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.

Revision history for this message
Alex Murray (alexmurray) wrote :

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?

Revision history for this message
kev (kbackhouse2000) wrote :

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

Revision history for this message
Alex Murray (alexmurray) wrote :

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.

Changed in whoopsie (Ubuntu):
importance: Undecided → High
Revision history for this message
Seth Arnold (seth-arnold) wrote :

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

Thanks

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

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.

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

New upload with the right file extension, same contents.

Changed in whoopsie (Ubuntu):
status: New → Triaged
Changed in whoopsie (Ubuntu):
assignee: nobody → Canonical Security Team (canonical-security)
Alex Murray (alexmurray)
Changed in whoopsie (Ubuntu):
assignee: Canonical Security Team (canonical-security) → Ubuntu Security Team (ubuntu-security)
Revision history for this message
Alex Murray (alexmurray) wrote :

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.

Changed in whoopsie (Ubuntu):
assignee: Ubuntu Security Team (ubuntu-security) → nobody
Revision history for this message
kev (kbackhouse2000) wrote :

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

Revision history for this message
kev (kbackhouse2000) wrote :

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

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

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

Revision history for this message
kev (kbackhouse2000) wrote :

Hi Tiago,

Thanks! The new patch looks good to me.

Kev

Revision history for this message
Alex Murray (alexmurray) wrote :

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

Revision history for this message
Alex Murray (alexmurray) wrote :

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?

Revision history for this message
Alex Murray (alexmurray) wrote :

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

Revision history for this message
kev (kbackhouse2000) wrote :

Thanks for letting me know.

Kev

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

Patch for eoan.

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

Patch for disco.

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

Patch for bionic.

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

Patch for xenial.

Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :
Revision history for this message
Tiago Stürmer Daitx (tdaitx) wrote :

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

Revision history for this message
Alex Murray (alexmurray) wrote :

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.

Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in whoopsie (Ubuntu):
status: Triaged → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :

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

Changed in whoopsie (Ubuntu):
status: Triaged → Fix Released
Alex Murray (alexmurray)
information type: Private Security → Public Security
tags: added: id-5d6412d0de485863a95da846
To post a comment you must log in.