hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-03-31 12:48:17 +02:00
Code Issues Packages Projects Releases Wiki Activity

let interprocedural analysis handle source-available extension methods for LogForgingLogMessageSink's

Browse Source
This commit is contained in:
Gregro
2026-03-21 19:30:31 +00:00
parent d0c48893f5
commit a59c865328
4 changed files with 13 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
csharp/ql/lib/change-notes/2026-03-19-fix-log-forging-extension-methods.md
View File

@@ -1,8 +1,7 @@
---
category: minorAnalysis
---
* The `cs/log-forging` query no longer treats arguments to user-defined extension methods
on `ILogger` types as sinks. Instead, taint is tracked interprocedurally through extension
method bodies, reducing false positives when extension methods sanitize input internally.
Known framework extension methods (`Microsoft.Extensions.Logging.LoggerExtensions`) are
now modeled as explicit sinks via Models as Data.
* The `cs/log-forging` query no longer treats arguments to extension methods with
source code on `ILogger` types as sinks. Instead, taint is tracked interprocedurally
through extension method bodies, reducing false positives when extension methods
sanitize input internally.

40
csharp/ql/lib/ext/Microsoft.Extensions.Logging.Sinks.model.yml
View File

@@ -1,40 +0,0 @@
extensions:
- addsTo:
pack: codeql/csharp-all
extensible: sinkModel
data:
# Log overloads
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "Log", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.LogLevel,Microsoft.Extensions.Logging.EventId,System.Exception,System.String,System.Object[])", "", "Argument[4..5]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "Log", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.LogLevel,Microsoft.Extensions.Logging.EventId,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "Log", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.LogLevel,System.Exception,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "Log", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.LogLevel,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
# LogCritical overloads
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogCritical", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.Exception,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogCritical", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogCritical", "(Microsoft.Extensions.Logging.ILogger,System.Exception,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogCritical", "(Microsoft.Extensions.Logging.ILogger,System.String,System.Object[])", "", "Argument[1..2]", "log-injection", "manual"]
# LogDebug overloads
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogDebug", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.Exception,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogDebug", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogDebug", "(Microsoft.Extensions.Logging.ILogger,System.Exception,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogDebug", "(Microsoft.Extensions.Logging.ILogger,System.String,System.Object[])", "", "Argument[1..2]", "log-injection", "manual"]
# LogError overloads
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogError", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.Exception,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogError", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogError", "(Microsoft.Extensions.Logging.ILogger,System.Exception,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogError", "(Microsoft.Extensions.Logging.ILogger,System.String,System.Object[])", "", "Argument[1..2]", "log-injection", "manual"]
# LogInformation overloads
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogInformation", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.Exception,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogInformation", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogInformation", "(Microsoft.Extensions.Logging.ILogger,System.Exception,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogInformation", "(Microsoft.Extensions.Logging.ILogger,System.String,System.Object[])", "", "Argument[1..2]", "log-injection", "manual"]
# LogTrace overloads
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogTrace", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.Exception,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogTrace", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogTrace", "(Microsoft.Extensions.Logging.ILogger,System.Exception,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogTrace", "(Microsoft.Extensions.Logging.ILogger,System.String,System.Object[])", "", "Argument[1..2]", "log-injection", "manual"]
# LogWarning overloads
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogWarning", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.Exception,System.String,System.Object[])", "", "Argument[3..4]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogWarning", "(Microsoft.Extensions.Logging.ILogger,Microsoft.Extensions.Logging.EventId,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogWarning", "(Microsoft.Extensions.Logging.ILogger,System.Exception,System.String,System.Object[])", "", "Argument[2..3]", "log-injection", "manual"]
- ["Microsoft.Extensions.Logging", "LoggerExtensions", false, "LogWarning", "(Microsoft.Extensions.Logging.ILogger,System.String,System.Object[])", "", "Argument[1..2]", "log-injection", "manual"]

12
csharp/ql/lib/semmle/code/csharp/security/dataflow/LogForgingQuery.qll
View File

@@ -10,7 +10,6 @@ private import semmle.code.csharp.frameworks.system.text.RegularExpressions
private import semmle.code.csharp.security.Sanitizers
private import semmle.code.csharp.security.dataflow.flowsinks.ExternalLocationSink
private import semmle.code.csharp.dataflow.internal.ExternalFlow
private import semmle.code.csharp.commons.Loggers
/**
* A data flow source for untrusted user input used in log entries.
@@ -54,13 +53,14 @@ private class HtmlSanitizer extends Sanitizer {
/**
* An argument to a call to a method on a logger class, excluding extension methods
* which are analyzed interprocedurally.
* with source code which are analyzed interprocedurally.
*/
private class LogForgingLogMessageSink extends Sink {
private class LogForgingLogMessageSink extends Sink, LogMessageSink {
LogForgingLogMessageSink() {
this.getExpr() = any(LoggerType i).getAMethod().getACall().getAnArgument() or
this.getExpr() =
any(MethodCall call | call.getQualifier().getType() instanceof LoggerType).getAnArgument()
not exists(ExtensionMethodCall mc |
this.getExpr() = mc.getAnArgument() and
mc.getTarget().fromSource()
)
}
}

7
csharp/ql/test/query-tests/Security Features/CWE-117/LogForging.expected
View File

@@ -7,17 +7,16 @@
edges
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:21:21:21:43 | ... + ... | provenance | |
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:31:50:31:72 | ... + ... | provenance | |
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:35:26:35:33 | access to local variable username | provenance | Sink:MaD:1 |
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:35:26:35:33 | access to local variable username | provenance | |
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:40:27:40:49 | ... + ... : String | provenance | |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:16:18:23 | access to local variable username : String | provenance | |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:27:18:61 | access to indexer : String | provenance | MaD:2 |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:27:18:61 | access to indexer : String | provenance | MaD:1 |
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:18:16:18:23 | access to local variable username : String | provenance | |
| LogForging.cs:40:27:40:49 | ... + ... : String | LogForging.cs:59:63:59:69 | message : String | provenance | |
| LogForging.cs:59:63:59:69 | message : String | LogForging.cs:61:21:61:27 | access to parameter message | provenance | |
| LogForgingAsp.cs:13:32:13:39 | username : String | LogForgingAsp.cs:17:21:17:43 | ... + ... | provenance | |
models
| 1 | Sink: Microsoft.Extensions.Logging; LoggerExtensions; false; LogError; (Microsoft.Extensions.Logging.ILogger,System.String,System.Object[]); ; Argument[1..2]; log-injection; manual |
| 2 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
| 1 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
nodes
| LogForging.cs:18:16:18:23 | access to local variable username : String | semmle.label | access to local variable username : String |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |