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

kill cursor of an internal only ClientCursor used for yielding could cause memory corruption

    • Type: Icon: Improvement Improvement
    • Resolution: Duplicate
    • Priority: Icon: Minor - P4 Minor - P4
    • None
    • Affects Version/s: None
    • Component/s: Security, Stability
    • None

      The cursor ids of the internal ClientCursors used when mongo operations yield the mutex should not be exposed externally. However, if these cursor id values are predictable and a kill cursors is issued against one while it is in use by a mongo operation it will delete the client cursor while in use, potentially leading to memory corruption.

      For example, in distinct.cpp a Cursor is created on line 82. A ClientCursor based on this Cursor is created on line 113. This ClientCursor does not exist to facilitate cursor based interaction with a client, but to leverage ClientCursor's ability to protect a Cursor when the db mutex is yielded. Using a ClientCursor in this manner is a common pattern in mongod code.

      The ClientCursor will be recorded in the ClientCursor::clientCursorsById registry of client cursors. If a client issues a killCursors command with the id of this ClientCursor, it will be destroyed and deallocated while still in use by the distinct.cpp implementation. This can result in distinct.cpp attempting to access invalid memory.

      The id of a ClientCursor created in distinct.cpp should not be explicitly provided to a client (although it can be printed in the server log). However, if the code for generating a ClientCursor id is predictable a malicious client could guess a ClientCursor's id and kill it while in use by distinct.cpp.

      The one place where client cursor ids are provided to a client is for use with get more. In this case, get more "pins" its ClientCursor to prevent deletion while in use. I believe we could relatively easily ensure that ClientCursors used internally (as in distinct.cpp) are also pinned, per comment in SERVER-4563. Longer term it might make sense to stop using the existing ClientCursor class for use cases unrelated to interaction with a client.

      ClientCursors are used throughout the code to protect a Cursor when the db mutex is yielded. And the same problem could occur in all of these places. I just picked distinct.cpp for the example because it is relatively straightforward to see how the Cursor and ClientCursor are used there.

            Assignee:
            david.storch@mongodb.com David Storch
            Reporter:
            aaron Aaron Staple
            Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

              Created:
              Updated:
              Resolved: