-
Type: Task
-
Resolution: Fixed
-
Priority: Major - P3
-
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.
- is related to
-
SERVER-82410 DocumentSourceListSearchIndexes should hold owned copy of command object
- Closed
- related to
-
SERVER-85313 Create a benchmark for BufBuilder::grow
- Closed