Bug 1627923 (CVE-2018-16790)

Created libbson tracking bugs for this issue:

Affects: epel-7 [bug 1627925]
Affects: fedora-all [bug 1627924]

libbson was merged into mongo-c-driver in Fedora ≥ 29.

Created attachment 1482654 [details]
Reproducer

Based on the upstream report.

Valgrind reports for libbson-1.9.5:

==9083== Conditional jump or move depends on uninitialised value(s)
==9083==    at 0x4859161: _bson_iter_next_internal (bson-iter.c:600)
==9083==    by 0x485A824: bson_iter_visit_all (bson-iter.c:1900)
==9083==    by 0x48572CF: _bson_as_json_visit_all (bson.c:3121)
==9083==    by 0x4012BF: main (in /tmp/a.out)

Valgrind reports for libbson bundled within mongo-c-driver-1.12.0:

==9084== Conditional jump or move depends on uninitialised value(s)
==9084==    at 0x4868829: _bson_iter_next_internal (bson-iter.c:634)
==9084==    by 0x486A454: bson_iter_visit_all (bson-iter.c:1934)
==9084==    by 0x4865BCA: _bson_as_json_visit_all (bson.c:3158)
==9084==    by 0x4012BF: main (in /tmp/a.out)

Both of them point uninitialized value read at:

      if (subtype == BSON_SUBTYPE_BINARY_DEPRECATED) {
         int32_t binary_len;

         if (l < 4) {
            iter->err_off = o;
            goto mark_invalid;
         }

         /* subtype 2 has a redundant length header in the data */
         memcpy (&binary_len, (iter->raw + iter->d3), sizeof (binary_len));
         binary_len = BSON_UINT32_FROM_LE (binary_len);
→        if (binary_len + 4 != l) {
            iter->err_off = iter->d3;
            goto mark_invalid;
         }
      }

While sanitizer reports a buffer overread at two different places:

      if (subtype == BSON_SUBTYPE_BINARY_DEPRECATED) {
         int32_t binary_len;

         if (l < 4) {
→           iter->err_off = o;
            goto mark_invalid;
         }

         /* subtype 2 has a redundant length header in the data */
→        memcpy (&binary_len, (iter->raw + iter->d3), sizeof (binary_len));
         binary_len = BSON_UINT32_FROM_LE (binary_len);
         if (binary_len + 4 != l) {
            iter->err_off = iter->d3;
            goto mark_invalid;
         }
      }

Reduced the reproducer to something a bit more actionable. Working upstream here: https://jira.mongodb.org/browse/CDRIVER-2819, but I'll post the comment here as well.

Looks like we're running off the end of a binary buffer.

Reproducer that triggers the ASAN error can be reduced to:
{0x11, 0x0, 0x0, 0x0, 0x5, 0xe, 0x19, 0x2e, 0x0, 0x4, 0x0, 0x0, 0x0, 0x2, 0x0, 0x0, 0x0}

which I believe decodes to:

0x11, 0x0, 0x0, 0x0 – Length of document is 0x11 (17) bytes
0x5, – binary data
0xe, 0x19, 0x2e, 0x0 – cstr
0x4, 0x0, 0x0, 0x0 – length of binary element (4)
0x2, – binary subtype (Binary (Old))
0x0, 0x0, 0x0, (Missing final byte)

Thus, if we change the final binary length to a 3 instead of a 4, we no longer trigger the out of bounds read.

Looking at the code now...

Upstream Patch:

https://github.com/mongodb/mongo-c-driver/commit/0d9a4d98bfdf4acd2c0138d4aaeb4e2e0934bd84

Created mongo-c-driver tracking bugs for this issue:

Affects: fedora-all [bug 1630042]

EPEL7's libbson-1.3.5 does not manifest this bug because bson_iter_next() does not read the redundant length header of BSON_SUBTYPE_BINARY_DEPRECATED by memcpy(). Otherwise the parser code is the same. I will apply the fix there. Making the parser stricter is not a bad thing.