Skip to content

Commit 3027ed2

Browse files
hvitvedhvitved
committed
C#: Include arguments to ILogger extension method calls in LogMessageSink 
1 parent 9dede31 commit 3027ed2

File tree

3 files changed

+28
-12
lines changed

3 files changed

+28
-12
lines changed

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/flowsinks/ExternalLocationSink.qll

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,14 @@ private class ExternalModelSink extends ExternalLocationSink {
2626
* An argument to a call to a method on a logger class.
2727
*/
2828
class LogMessageSink extends ExternalLocationSink {
29-
LogMessageSink() { this.getExpr() = any(LoggerType i).getAMethod().getACall().getAnArgument() }
29+
LogMessageSink() {
30+
this.getExpr() = any(LoggerType i).getAMethod().getACall().getAnArgument()
31+
or
32+
this.getExpr() =
33+
any(ExtensionMethodCall call |
34+
call.getTarget().(ExtensionMethod).getExtendedType() instanceof LoggerType
35+
).getArgument(any(int i | i > 0))
36+
}
3037
}
3138

3239
/**

‎csharp/ql/test/query-tests/Security Features/CWE-117/LogForging.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.IO;
44
using System.Net;
55
using System.Web;
6+
using Microsoft.Extensions.Logging;
67

78
class ILogger
89
{
@@ -24,6 +25,10 @@ public void ProcessRequest(HttpContext ctx)
2425
logger.Warn(WebUtility.HtmlEncode(username) + " logged in");
2526
// BAD: Logged as-is to TraceSource
2627
new TraceSource("Test").TraceInformation(username + " logged in");
28+
29+
Microsoft.Extensions.Logging.ILogger logger2 = null;
30+
// BAD: Logged as-is
31+
logger2.LogError(username);
2732
}
2833

2934
public bool IsReusable

‎csharp/ql/test/query-tests/Security Features/CWE-117/LogForging.expected

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,23 @@
11
edges
2-
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:17:27:17:61 | access to indexer : String |
3-
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:20:21:20:43 | ... + ... |
4-
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | LogForging.cs:26:50:26:72 | ... + ... |
5-
| LogForging.cs:17:27:17:61 | access to indexer : String | LogForging.cs:20:21:20:43 | ... + ... |
6-
| LogForging.cs:17:27:17:61 | access to indexer : String | LogForging.cs:26:50:26:72 | ... + ... |
2+
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:27:18:61 | access to indexer : String |
3+
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:21:21:21:43 | ... + ... |
4+
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:27:50:27:72 | ... + ... |
5+
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:31:26:31:33 | access to local variable username |
6+
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:21:21:21:43 | ... + ... |
7+
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:27:50:27:72 | ... + ... |
8+
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:31:26:31:33 | access to local variable username |
79
| LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... |
810
nodes
9-
| LogForging.cs:17:27:17:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
10-
| LogForging.cs:17:27:17:61 | access to indexer : String | semmle.label | access to indexer : String |
11-
| LogForging.cs:20:21:20:43 | ... + ... | semmle.label | ... + ... |
12-
| LogForging.cs:26:50:26:72 | ... + ... | semmle.label | ... + ... |
11+
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection |
12+
| LogForging.cs:18:27:18:61 | access to indexer : String | semmle.label | access to indexer : String |
13+
| LogForging.cs:21:21:21:43 | ... + ... | semmle.label | ... + ... |
14+
| LogForging.cs:27:50:27:72 | ... + ... | semmle.label | ... + ... |
15+
| LogForging.cs:31:26:31:33 | access to local variable username | semmle.label | access to local variable username |
1316
| LogForgingAsp.cs:8:32:8:39 | username : String | semmle.label | username : String |
1417
| LogForgingAsp.cs:12:21:12:43 | ... + ... | semmle.label | ... + ... |
1518
subpaths
1619
#select
17-
| 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 |
18-
| 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 |
20+
| 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 |
21+
| 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 |
22+
| 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 |
1923
| 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 |

0 commit comments

Comments
 (0)