Skip to content

Commit 030a238

Browse files
committed
C#: Recognize safe output encoding in cs/log-forging
1 parent 1912c4a commit 030a238

46 files changed

Lines changed: 1674 additions & 0 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/JsonLoggingConfiguration.qll‎

Lines changed: 782 additions & 0 deletions
Large diffs are not rendered by default.

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/LogForgingQuery.qll‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ private import semmle.code.csharp.frameworks.system.text.RegularExpressions
1010
private import semmle.code.csharp.security.Sanitizers
1111
private import semmle.code.csharp.security.dataflow.flowsinks.ExternalLocationSink
1212
private import semmle.code.csharp.dataflow.internal.ExternalFlow
13+
private import semmle.code.csharp.security.dataflow.JsonLoggingConfiguration
1314

1415
/**
1516
* A data flow source for untrusted user input used in log entries.
@@ -57,6 +58,7 @@ private class HtmlSanitizer extends Sanitizer {
5758
*/
5859
private class LogForgingLogMessageSink extends Sink, LogMessageSink {
5960
LogForgingLogMessageSink() {
61+
not isJsonProtectedLogArgument(this.getExpr()) and
6062
not exists(ExtensionMethodCall mc |
6163
this.getExpr() = mc.getAnArgument() and
6264
mc.getTarget().fromSource()
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cs/log-forging` query now uses a conservative, database-wide heuristic to recognize
5+
supported code-configured Serilog and .NET JSON-only logging setups. This reduces false-positive
6+
results for standard logging calls when all visible logging configuration in the database is
7+
safe and understood.
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#select
2+
edges
3+
nodes
4+
subpaths
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
query: Security Features/CWE-117/LogForging.ql
2+
postprocess:
3+
- utils/test/PrettyPrintModels.ql
4+
- utils/test/InlineExpectationsTestQuery.ql
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using Microsoft.AspNetCore.Mvc;
2+
using Microsoft.Extensions.Logging;
3+
using System.Web;
4+
5+
class BuiltInJsonLoggingController : ControllerBase
6+
{
7+
public void Action(HttpContext context)
8+
{
9+
using var factory = LoggerFactory.Create(logging => logging.AddJsonConsole());
10+
var logger = factory.CreateLogger("Example");
11+
var genericLogger = factory.CreateLogger<BuiltInJsonLoggingController>();
12+
string input = context.Request.QueryString["input"];
13+
logger.LogInformation("Input: {Input}", input);
14+
logger.LogInformation($"Input: {input}");
15+
genericLogger.LogInformation("Input: {Input}", input);
16+
17+
var explicitlyDisposed = LoggerFactory.Create(logging => logging.AddJsonConsole());
18+
explicitlyDisposed.Dispose();
19+
}
20+
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
semmle-extractor-options: /nostdlib /noconfig
2+
semmle-extractor-options: --load-sources-from-project:../../../resources/stubs/_frameworks/Microsoft.AspNetCore.App/Microsoft.AspNetCore.App.csproj
3+
semmle-extractor-options: ${testdir}/../../../resources/stubs/System.Web.cs
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#select
2+
| Test.cs:16:49:16:53 | access to local variable input | Test.cs:15:24:15:50 | access to property QueryString : NameValueCollection | Test.cs:16:49:16:53 | access to local variable input | This log entry depends on a $@. | Test.cs:15:24:15:50 | access to property QueryString | user-provided value |
3+
edges
4+
| Test.cs:15:16:15:20 | access to local variable input : String | Test.cs:16:49:16:53 | access to local variable input | provenance | |
5+
| Test.cs:15:24:15:50 | access to property QueryString : NameValueCollection | Test.cs:15:16:15:20 | access to local variable input : String | provenance | |
6+
| Test.cs:15:24:15:50 | access to property QueryString : NameValueCollection | Test.cs:15:24:15:59 | access to indexer : String | provenance | MaD:1 |
7+
| Test.cs:15:24:15:59 | access to indexer : String | Test.cs:15:16:15:20 | access to local variable input : String | provenance | |
8+
models
9+
| 1 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
10+
nodes
11+
| Test.cs:15:16:15:20 | access to local variable input : String | semmle.label | access to local variable input : String |
12+
| Test.cs:15:24:15:50 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
13+
| Test.cs:15:24:15:59 | access to indexer : String | semmle.label | access to indexer : String |
14+
| Test.cs:16:49:16:53 | access to local variable input | semmle.label | access to local variable input |
15+
subpaths
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
query: Security Features/CWE-117/LogForging.ql
2+
postprocess:
3+
- utils/test/PrettyPrintModels.ql
4+
- utils/test/InlineExpectationsTestQuery.ql
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
using Microsoft.AspNetCore.Mvc;
2+
using Microsoft.Extensions.Logging;
3+
using System.Web;
4+
5+
class MixedBuiltInLoggingController : ControllerBase
6+
{
7+
public void Configure()
8+
{
9+
LoggerFactory.Create(logging => logging.ClearProviders().AddJsonConsole());
10+
LoggerFactory.Create(logging => logging.AddConsole());
11+
}
12+
13+
public void Action(HttpContext context, ILogger logger)
14+
{
15+
string input = context.Request.QueryString["input"]; // $ Source
16+
logger.LogInformation("Input: {Input}", input); // $ Alert
17+
}
18+
}

0 commit comments

Comments
 (0)