Uploaded image for project: 'Core Server'
  1. Core Server
  2. SERVER-83137

Consider additional validation of size args to BufBuilder::appendBuf

    • Type: Icon: Task Task
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 7.3.0-rc0
    • Affects Version/s: None
    • Component/s: None
    • Storage Execution
    • Fully Compatible
    • Execution Team 2023-12-25, Execution Team 2024-01-08, Execution Team 2024-01-22

      BufBuilder::appendBuf (https://github.com/mongodb/mongo/blob/817bd0871e4d269c09c4a7ed009fba790549288e/src/mongo/bson/util/builder.h#L399C4-L402) accepts a size_t len argument describing the number of bytes to copy from the src buffer.

      Before beginning the memcpy, it C-style casts len to `int`, and then compares that integer to the current buffer size to see if it needs to grow the current buffer.

      When a large-enough size_t is passed to appendBuf, this can result in the C-style cast producing a negative number, and therefore the growth-needed-check receiving a negative number and concluding that the buffer doesn't need to grow. This will result in the memcpy writing to arbitrary addresses in the heap outside the range of the current buffer.

      A motivating case of how this might happen is someone using BSONObjBuilder::append (see https://github.com/mongodb/mongo/blob/817bd0871e4d269c09c4a7ed009fba790549288e/src/mongo/bson/bsonobjbuilder.h#L180) to append a BSONObj. If the BSONObj we're appending is corrupt/has already been freed, it's objsize() member may return a negative integer, but because it will be converted to size_t in the call to appendBuf but back to a negative integer during the bounds-check, we won't catch the issue before we start copying the garbage memory into addresses outside the BufBuilder's ownership.

      If it's possible without sacrificing performance (maybe only in debug builds?), we could add a check that the length arguments passed to appendBuf have positive `int` representations, and fail the appendBuf call if not. This would allow us to catch invalid calls to appendBuf before it further corrupts the heap.

            Assignee:
            wei.hu@mongodb.com Wei Hu
            Reporter:
            george.wangensteen@mongodb.com George Wangensteen
            Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

              Created:
              Updated:
              Resolved: