Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create rule S5147(C#): NoSQL operations should not be vulnerable to injection attacks APPSEC-2024 #4165

Merged
merged 8 commits into from
Aug 23, 2024
9 changes: 9 additions & 0 deletions rules/S5147/common/fix/builder-pattern.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
==== Use safe builder patterns

Generally, database queries also accept builder patterns to build queries. This
is a safe way to build queries as it ensures that the query is built correctly
and is safe from injection attacks because it does not require you to ensure
that the query is built correctly.

For example, using a `.where()` function instead of a string and `$where` will
help avoid an injection attack.
77 changes: 77 additions & 0 deletions rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
== How to fix it in MongoDB

=== Code examples

The following code is vulnerable to NoSQL injections because untrusted data is
used to find data in a database.
Such cases can be encountered when client-side code crafts the query, such as
``++[{ '$match': { 'Username': 'John Doe' } }]++``.

Note that `Find` and `FindAsync` are not the only constructs whose input should be
verified. Multiple
https://mongodb.github.io/mongo-csharp-driver/2.4/reference/driver/definitions/[definitions]
can be built from a string and allow attackers to leak or tamper with data.

==== Noncompliant code example

[source,csharp,diff-id=1,diff-type=noncompliant]
----
using MongoDB.Driver;
using MongoDB.Bson;
[ApiController]
[Route("Example")]
public class ExampleController: ControllerBase
{
private string connectionString;
[Route("Example")]
public async Task<string> Example()
{
var client = new MongoClient(connectionString);
var database = client.GetDatabase("example");
var collection = database.GetCollection<Message>("messages");
var filterDefinition = Request.Query["filterDefinition"];
await collection.FindAsync(filter)
}
}
----

==== Compliant solution

[source,csharp,diff-id=1,diff-type=compliant]
----
using MongoDB.Driver;
using MongoDB.Bson;
[ApiController]
[Route("Example")]
public class ExampleController: ControllerBase
{
private string connectionString;
[Route("Example")]
public async Task<string> Example()
{
var client = new MongoClient(connectionString);
var database = client.GetDatabase("example");
var collection = database.GetCollection<Message>("messages");
var filterDefinition = Builders<BsonDocument>.Filter.Eq("Username", "Example");
await collection.FindAsync(filter)
}
}
----

=== How does this work?

include::../../common/fix/builder-pattern.adoc[]

If using a builder pattern is not possible, follow the instructions below:

include::../../common/fix/pre-approved-list.adoc[]

include::../../common/fix/dangerous-operators.adoc[]
2 changes: 2 additions & 0 deletions rules/S5147/csharp/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}
27 changes: 27 additions & 0 deletions rules/S5147/csharp/rule.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
== Why is this an issue?

include::../rationale.adoc[]

include::../impact.adoc[]

include::how-to-fix-it/mongodb-csharp-driver.adoc[]

== Resources

include::../common/resources/articles.adoc[]

include::../common/resources/standards.adoc[]

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

include::../message.adoc[]

include::../highlighting.adoc[]

'''

endif::env-github,rspecator-view[]
Loading