bugfix: possible nosql injection

[matrix0123456789]

You can make any comparsion by an equal operator if you compare to BsonDocument for example:

IMongoCollection<BsonDocument> collection;
var value = new BsonDocument { { "$gt", 3 } };
var query = collection.Find(x => x["field"] == value);

//it will execute:
//find({ "field" : { "$gt" : 3 } })

I think it can cause security vulnerability for unaware developer, who trust, that equal operator always will test for equality:

public List<BsonDocument> GetObjectByUser(IMongoCollection<BsonDocument> collection, BsonValue data)
{
	return collection.Find(x => x["userId"] == data["userId"]).ToList();
}
public void Attack(IMongoCollection<BsonDocument> collection)
{
	var data = GetObjectByUser(collection, new BsonDocument { { "userId", new BsonDocument { { "$ne", ObjectId.Empty } } } });
}

[vincentkam]

Thank you for your PR! I've created https://jira.mongodb.org/browse/CSHARP-3080 to track this

[vincentkam]

Hello @matrix0123456789,

Once again, thank you for your PR! The MongoDB .NET Driver team and the MongoDB Security team has discussed this vulnerability in depth. At a high-level, the vulnerability is a result of the fact that the Mongo Query Language (MQL) uses an implicit $eq operator. This is simply a property of our query language, and it makes it much simpler for hundreds of applications out there to write simple eq queries.

E.g. { x: 1 } and { x: {$eq: 1} } are the same because MQL implicitly uses the $eq operator.

We understand that you believe that an attacker can construct a specific JSON file which will take advantage of this fact to construct a NoSQL injection (or at least a different query that was originally intended).

We believe this does NOT warrant a CVE or a change to the driver due to the following:

1. This is essentially an application layer problem. An application should anticipate that the implicit equality operator is always used. For example, let us imagine there is a malicious JSON file with this injection. The driver does not parse this file - this is done by the application. Therefore, it is the responsibility of the application to handle these types of issues e.g. by sanitizing the input prior to passing it to the driver.

2. Thus, the .NET driver is doing what is supposed to do (i.e. using the implicit $eq parameter, which is a property of MQL).

As a point of comparison, broadly speaking, SQL injection does not really have a CVE because it is a property of the language and if the application is not sanitising user input properly, then it is unfortunately the fault of the application, not the query language. This NoSQL injection described by your PR is quite similar in that respect.

Unfortunately, there is no way this could be fixed by us, apart from changing the behaviour of query language, which would make hundreds of apps stop working

Nevertheless, we will definitely improve our documentation so that developers are made more aware of these potential issues. The implicit $eq parameter is mentioned in our documentation, but perhaps not as clearly as it should.

Please feel free to reach out with any additional questions or concerns that you might have!