hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-25 16:55:19 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #13183 from hvitved/csharp/ilogger-extension-methods

Browse Source 
C#: Include arguments to `ILogger` extension method calls in `LogMessageSink`
This commit is contained in:
Tom Hvitved
2023-05-17 08:20:57 +02:00
committed by GitHub
parent 2dcdc71e45 406acbe6a4
commit 867bdcf74d
4 changed files with 32 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
csharp/ql/lib/change-notes/2023-05-16-ilogger-extension-methods.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `cs/log-forging`, `cs/cleartext-storage`, and `cs/exposure-of-sensitive-information` queries now correctly handle unsanitized arguments to `ILogger` extension methods.

9
csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll
View File

@@ -26,7 +26,14 @@ private class ExternalModelSink extends ExternalLocationSink {
* An argument to a call to a method on a logger class.
*/
class LogMessageSink extends ExternalLocationSink {
LogMessageSink() { this.getExpr() = any(LoggerType i).getAMethod().getACall().getAnArgument() }
LogMessageSink() {
this.getExpr() = any(LoggerType i).getAMethod().getACall().getAnArgument()
or
this.getExpr() =
any(ExtensionMethodCall call |
call.getTarget().(ExtensionMethod).getExtendedType() instanceof LoggerType
).getArgument(any(int i | i > 0))
}
}
/**

5
csharp/ql/test/query-tests/Security Features/CWE-117/LogForging.cs
View File

@@ -3,6 +3,7 @@ using System.Diagnostics;
using System.IO;
using System.Net;
using System.Web;
using Microsoft.Extensions.Logging;
class ILogger
{
@@ -24,6 +25,10 @@ public class LogForgingHandler : IHttpHandler
logger.Warn(WebUtility.HtmlEncode(username) + " logged in");
// BAD: Logged as-is to TraceSource
new TraceSource("Test").TraceInformation(username + " logged in");
Microsoft.Extensions.Logging.ILogger logger2 = null;
// BAD: Logged as-is
logger2.LogError(username);
}
public bool IsReusable

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

@@ -1,19 +1,23 @@
edges
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:17:27:17:61 | access to indexer : String |
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:20:21:20:43 | ... + ... |
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:26:50:26:72 | ... + ... |
| LogForging.cs:17:27:17:61 | access to indexer : String | LogForging.cs:20:21:20:43 | ... + ... |
| LogForging.cs:17:27:17:61 | access to indexer : String | LogForging.cs:26:50:26:72 | ... + ... |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:27:18:61 | access to indexer : String |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:21:21:21:43 | ... + ... |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:27:50:27:72 | ... + ... |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:31:26:31:33 | access to local variable username |
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:21:21:21:43 | ... + ... |
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:27:50:27:72 | ... + ... |
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:31:26:31:33 | access to local variable username |
| LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... |
nodes
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
| LogForging.cs:17:27:17:61 | access to indexer : String | semmle.label | access to indexer : String |
| LogForging.cs:20:21:20:43 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:26:50:26:72 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
| LogForging.cs:18:27:18:61 | access to indexer : String | semmle.label | access to indexer : String |
| LogForging.cs:21:21:21:43 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:27:50:27:72 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:31:26:31:33 | access to local variable username | semmle.label | access to local variable username |
| LogForgingAsp.cs:8:32:8:39 | username : String | semmle.label | username : String |
| LogForgingAsp.cs:12:21:12:43 | ... + ... | semmle.label | ... + ... |
subpaths
#select
| LogForging.cs:20:21:20:43 | ... + ... | LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:20:21:20:43 | ... + ... | This log entry depends on a $@. | LogForging.cs:17:27:17:49 | access to property QueryString | user-provided value |
| LogForging.cs:26:50:26:72 | ... + ... | LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:26:50:26:72 | ... + ... | This log entry depends on a $@. | LogForging.cs:17:27:17:49 | access to property QueryString | user-provided value |
| LogForging.cs:21:21:21:43 | ... + ... | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:21:21:21:43 | ... + ... | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
| LogForging.cs:27:50:27:72 | ... + ... | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:27:50:27:72 | ... + ... | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
| LogForging.cs:31:26:31:33 | access to local variable username | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:31:26:31:33 | access to local variable username | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
| LogForgingAsp.cs:12:21:12:43 | ... + ... | LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... | This log entry depends on a $@. | LogForgingAsp.cs:8:32:8:39 | username | user-provided value |