From 25d4065a6543032a67fbdfbe02fc370541fab3c2 Mon Sep 17 00:00:00 2001 From: loris-s-sonarsource Date: Tue, 20 Aug 2024 11:13:39 +0000 Subject: [PATCH 1/7] Add csharp to rule S5147 --- rules/S5147/csharp/metadata.json | 2 ++ rules/S5147/csharp/rule.adoc | 44 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 rules/S5147/csharp/metadata.json create mode 100644 rules/S5147/csharp/rule.adoc diff --git a/rules/S5147/csharp/metadata.json b/rules/S5147/csharp/metadata.json new file mode 100644 index 00000000000..7a73a41bfdf --- /dev/null +++ b/rules/S5147/csharp/metadata.json @@ -0,0 +1,2 @@ +{ +} \ No newline at end of file diff --git a/rules/S5147/csharp/rule.adoc b/rules/S5147/csharp/rule.adoc new file mode 100644 index 00000000000..e2c7df430f3 --- /dev/null +++ b/rules/S5147/csharp/rule.adoc @@ -0,0 +1,44 @@ +FIXME: add a description + +// If you want to factorize the description uncomment the following line and create the file. +//include::../description.adoc[] + +== Why is this an issue? + +FIXME: remove the unused optional headers (that are commented out) + +//=== What is the potential impact? + +== How to fix it +//== How to fix it in FRAMEWORK NAME + +=== Code examples + +==== Noncompliant code example + +[source,csharp,diff-id=1,diff-type=noncompliant] +---- +FIXME +---- + +==== Compliant solution + +[source,csharp,diff-id=1,diff-type=compliant] +---- +FIXME +---- + +//=== How does this work? + +//=== Pitfalls + +//=== Going the extra mile + + +//== Resources +//=== Documentation +//=== Articles & blog posts +//=== Conference presentations +//=== Standards +//=== External coding guidelines +//=== Benchmarks From f2280ca106eee8f8fee41e1649c5e2e1c3965baa Mon Sep 17 00:00:00 2001 From: Loris Sierra Date: Tue, 20 Aug 2024 18:11:07 +0200 Subject: [PATCH 2/7] Add the text --- .../header_names/allowed_framework_names.adoc | 1 + rules/S5147/common/fix/builder-pattern.adoc | 9 +++ .../how-to-fix-it/mongo-csharp-driver.adoc | 77 +++++++++++++++++++ rules/S5147/csharp/rule.adoc | 45 ++++------- 4 files changed, 101 insertions(+), 31 deletions(-) create mode 100644 rules/S5147/common/fix/builder-pattern.adoc create mode 100644 rules/S5147/csharp/how-to-fix-it/mongo-csharp-driver.adoc diff --git a/docs/header_names/allowed_framework_names.adoc b/docs/header_names/allowed_framework_names.adoc index 029fad01377..1ce99241f5c 100644 --- a/docs/header_names/allowed_framework_names.adoc +++ b/docs/header_names/allowed_framework_names.adoc @@ -8,6 +8,7 @@ * Dapper * BouncyCastle * Jwt.Net +* MongoDB C# Driver // C-Family * Botan * CryptoPP diff --git a/rules/S5147/common/fix/builder-pattern.adoc b/rules/S5147/common/fix/builder-pattern.adoc new file mode 100644 index 00000000000..2df3c56866d --- /dev/null +++ b/rules/S5147/common/fix/builder-pattern.adoc @@ -0,0 +1,9 @@ +==== Use safe builder patterns + +Generally, Databases 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. diff --git a/rules/S5147/csharp/how-to-fix-it/mongo-csharp-driver.adoc b/rules/S5147/csharp/how-to-fix-it/mongo-csharp-driver.adoc new file mode 100644 index 00000000000..f7c27b95846 --- /dev/null +++ b/rules/S5147/csharp/how-to-fix-it/mongo-csharp-driver.adoc @@ -0,0 +1,77 @@ +== How to fix it in MongoDB C# Driver + +=== 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 +{ + [Route("Example")] + public async Task Noncompliant() + { + var connectionString = Environment.GetEnvironmentVariable("MONGODB_URI"); + + var client = new MongoClient(connectionString); + var database = client.GetDatabase("demo"); + var collection = database.GetCollection("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 +{ + [Route("Example")] + public async Task Noncompliant() + { + var connectionString = Environment.GetEnvironmentVariable("MONGODB_URI"); + + var client = new MongoClient(connectionString); + var database = client.GetDatabase("demo"); + var collection = database.GetCollection("messages"); + + var filterDefinition = Builders.Filter.Eq("Username", "John Doe"); + + 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[] diff --git a/rules/S5147/csharp/rule.adoc b/rules/S5147/csharp/rule.adoc index e2c7df430f3..728c0609f15 100644 --- a/rules/S5147/csharp/rule.adoc +++ b/rules/S5147/csharp/rule.adoc @@ -1,44 +1,27 @@ -FIXME: add a description - -// If you want to factorize the description uncomment the following line and create the file. -//include::../description.adoc[] - == Why is this an issue? -FIXME: remove the unused optional headers (that are commented out) - -//=== What is the potential impact? +include::../rationale.adoc[] -== How to fix it -//== How to fix it in FRAMEWORK NAME +include::../impact.adoc[] -=== Code examples +include::how-to-fix-it/mongodb-csharp-driver.adoc[] -==== Noncompliant code example +== Resources -[source,csharp,diff-id=1,diff-type=noncompliant] ----- -FIXME ----- +include::../common/resources/articles.adoc[] -==== Compliant solution +include::../common/resources/standards.adoc[] -[source,csharp,diff-id=1,diff-type=compliant] ----- -FIXME ----- +ifdef::env-github,rspecator-view[] -//=== How does this work? +''' +== Implementation Specification +(visible only on this page) -//=== Pitfalls +include::../message.adoc[] -//=== Going the extra mile +include::../highlighting.adoc[] +''' -//== Resources -//=== Documentation -//=== Articles & blog posts -//=== Conference presentations -//=== Standards -//=== External coding guidelines -//=== Benchmarks +endif::env-github,rspecator-view[] From cf837b3b93d2d27864c6dea6a70b1f852a09b467 Mon Sep 17 00:00:00 2001 From: Loris Sierra Date: Tue, 20 Aug 2024 18:15:30 +0200 Subject: [PATCH 3/7] Fixed filename --- .../{mongo-csharp-driver.adoc => mongodb-csharp-driver.adoc} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename rules/S5147/csharp/how-to-fix-it/{mongo-csharp-driver.adoc => mongodb-csharp-driver.adoc} (100%) diff --git a/rules/S5147/csharp/how-to-fix-it/mongo-csharp-driver.adoc b/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc similarity index 100% rename from rules/S5147/csharp/how-to-fix-it/mongo-csharp-driver.adoc rename to rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc From e97e47f72c68eb567d963456601db791e5ac3730 Mon Sep 17 00:00:00 2001 From: "Loris S." <91723853+loris-s-sonarsource@users.noreply.github.com> Date: Thu, 22 Aug 2024 17:59:27 +0200 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Hendrik Buchwald <64110887+hendrik-buchwald-sonarsource@users.noreply.github.com> --- .../how-to-fix-it/mongodb-csharp-driver.adoc | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc b/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc index f7c27b95846..b0b3bcde432 100644 --- a/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc +++ b/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc @@ -1,15 +1,15 @@ -== How to fix it in MongoDB C# Driver +== 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' } }]`++. +++``[{ '$match': { 'Username': 'John Doe' } }]``++. -Note that Find and FindAsync are not the only constructs whose input should be +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] +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 @@ -24,12 +24,11 @@ using MongoDB.Bson; public class ExampleController: ControllerBase { [Route("Example")] - public async Task Noncompliant() + public async Task Example() { - var connectionString = Environment.GetEnvironmentVariable("MONGODB_URI"); var client = new MongoClient(connectionString); - var database = client.GetDatabase("demo"); + var database = client.GetDatabase("example"); var collection = database.GetCollection("messages"); var filterDefinition = Request.Query["filterDefinition"]; @@ -51,15 +50,14 @@ using MongoDB.Bson; public class ExampleController: ControllerBase { [Route("Example")] - public async Task Noncompliant() + public async Task Example() { - var connectionString = Environment.GetEnvironmentVariable("MONGODB_URI"); var client = new MongoClient(connectionString); - var database = client.GetDatabase("demo"); + var database = client.GetDatabase("example"); var collection = database.GetCollection("messages"); - var filterDefinition = Builders.Filter.Eq("Username", "John Doe"); + var filterDefinition = Builders.Filter.Eq("Username", "Example"); await collection.FindAsync(filter) } From f33a5635effb0e14abd647a544e39af98fdabb1c Mon Sep 17 00:00:00 2001 From: "Loris S." <91723853+loris-s-sonarsource@users.noreply.github.com> Date: Thu, 22 Aug 2024 18:01:08 +0200 Subject: [PATCH 5/7] Apply suggestions from code review --- rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc b/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc index b0b3bcde432..2db3264078f 100644 --- a/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc +++ b/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc @@ -23,10 +23,11 @@ using MongoDB.Bson; [Route("Example")] public class ExampleController: ControllerBase { + private string connectionString; + [Route("Example")] public async Task Example() { - var client = new MongoClient(connectionString); var database = client.GetDatabase("example"); var collection = database.GetCollection("messages"); @@ -49,10 +50,11 @@ using MongoDB.Bson; [Route("Example")] public class ExampleController: ControllerBase { + private string connectionString; + [Route("Example")] public async Task Example() { - var client = new MongoClient(connectionString); var database = client.GetDatabase("example"); var collection = database.GetCollection("messages"); From 52c1c058ee36666449951cb158d610b10656dd8b Mon Sep 17 00:00:00 2001 From: "Loris S." <91723853+loris-s-sonarsource@users.noreply.github.com> Date: Thu, 22 Aug 2024 18:02:54 +0200 Subject: [PATCH 6/7] Update rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc --- rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc b/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc index 2db3264078f..d862a366df6 100644 --- a/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc +++ b/rules/S5147/csharp/how-to-fix-it/mongodb-csharp-driver.adoc @@ -5,7 +5,7 @@ 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' } }]``++. +``++[{ '$match': { 'Username': 'John Doe' } }]++``. Note that `Find` and `FindAsync` are not the only constructs whose input should be verified. Multiple From cd5b383f68bcb4c4f9bd7aceeabafcc2aa93a53e Mon Sep 17 00:00:00 2001 From: "Loris S." <91723853+loris-s-sonarsource@users.noreply.github.com> Date: Fri, 23 Aug 2024 14:06:23 +0200 Subject: [PATCH 7/7] Update rules/S5147/common/fix/builder-pattern.adoc Co-authored-by: Hendrik Buchwald <64110887+hendrik-buchwald-sonarsource@users.noreply.github.com> --- rules/S5147/common/fix/builder-pattern.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/S5147/common/fix/builder-pattern.adoc b/rules/S5147/common/fix/builder-pattern.adoc index 2df3c56866d..bc05f38dc5f 100644 --- a/rules/S5147/common/fix/builder-pattern.adoc +++ b/rules/S5147/common/fix/builder-pattern.adoc @@ -1,6 +1,6 @@ ==== Use safe builder patterns -Generally, Databases queries also accept builder patterns to build queries. This +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.