[SERVER-8369] kill cursor of an internal only ClientCursor used for yielding could cause memory corruption Created: 29/Jan/13  Updated: 24/Jan/17  Resolved: 24/Jan/17

Status: Closed
Project: Core Server
Component/s: Security, Stability
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor - P4
Reporter: Aaron Staple Assignee: David Storch
Resolution: Duplicate Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
duplicates SERVER-27065 Segfault in building an aggregation Closed
Related
is related to SERVER-4563 simplify ClientCursor lifecycle Closed
is related to SERVER-9609 Ensure users can only call getMore on... Closed
Participants:

 Description   

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.



 Comments   
Comment by David Storch [ 24/Jan/17 ]

This was fixed under SERVER-27065 by commit https://github.com/mongodb/mongo/commit/1e8f34fc476705888f7dec8d06c780de4e556988. As part of this commit, cursors come into existence pinned. There is no longer a time window in between ClientCursor creation and pinning during which the ClientCursor could be killed out from other the pinning thread. Resolving as a duplicate.

Comment by Dwight Merriman [ 13/Mar/14 ]

does the existence of Runner mean this is resolved?

Generated at Thu Feb 08 03:17:15 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.