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

Merge pull request #15212 from michaelnebel/csharp/stringreplace

Browse Source 
C#: Fix Log forging false positive.
This commit is contained in:
Michael Nebel
2024-01-04 13:38:29 +01:00
committed by GitHub
parent 7c6d30b1a2 0c78cccf62
commit 682aef9548
4 changed files with 16 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
csharp/ql/lib/semmle/code/csharp/frameworks/System.qll
View File

@@ -346,11 +346,11 @@ class SystemStringClass extends StringType {
result.hasName("==") result.hasName("==")
} }
/** Gets the `Replace(string/char, string/char)` method. */ /** Gets the `Replace(...)` method. */
Method getReplaceMethod() { Method getReplaceMethod() {
result.getDeclaringType() = this and result.getDeclaringType() = this and
result.hasName("Replace") and result.hasName("Replace") and
result.getNumberOfParameters() = 2 and result.getNumberOfParameters() in [2 .. 4] and
result.getReturnType() instanceof StringType result.getReturnType() instanceof StringType
} }

4
csharp/ql/src/change-notes/2024-01-03-logforging-false-positive.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Fixed a Log forging false positive when using `String.Replace` to sanitize the input.

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

@@ -21,6 +21,8 @@ public class LogForgingHandler : IHttpHandler
logger.Warn(username + " logged in"); logger.Warn(username + " logged in");
// GOOD: New-lines removed // GOOD: New-lines removed
logger.Warn(username.Replace(Environment.NewLine, "") + " logged in"); logger.Warn(username.Replace(Environment.NewLine, "") + " logged in");
// GOOD: New-lines removed
logger.Warn(username.Replace(Environment.NewLine, "", StringComparison.InvariantCultureIgnoreCase) + " logged in");
// GOOD: Html encoded // GOOD: Html encoded
logger.Warn(WebUtility.HtmlEncode(username) + " logged in"); logger.Warn(WebUtility.HtmlEncode(username) + " logged in");
// BAD: Logged as-is to TraceSource // BAD: Logged as-is to TraceSource

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

@@ -1,23 +1,23 @@
edges edges
| 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: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: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:29:50:29: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:49 | access to property QueryString : NameValueCollection | LogForging.cs:33:26:33: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: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:29:50:29:72 | ... + ... |
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:31:26:31:33 | access to local variable username | | LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:33:26:33:33 | access to local variable username |
| LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... | | LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... |
nodes nodes
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | semmle.label | access to property QueryString : NameValueCollection | | 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:18:27:18:61 | access to indexer : String | semmle.label | access to indexer : String |
| LogForging.cs:21:21:21:43 | ... + ... | semmle.label | ... + ... | | LogForging.cs:21:21:21:43 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:27:50:27:72 | ... + ... | semmle.label | ... + ... | | LogForging.cs:29:50:29:72 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:31:26:31:33 | access to local variable username | semmle.label | access to local variable username | | LogForging.cs:33:26:33: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:8:32:8:39 | username : String | semmle.label | username : String |
| LogForgingAsp.cs:12:21:12:43 | ... + ... | semmle.label | ... + ... | | LogForgingAsp.cs:12:21:12:43 | ... + ... | semmle.label | ... + ... |
subpaths subpaths
#select #select
| 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: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:29:50:29:72 | ... + ... | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:29:50:29: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 | | LogForging.cs:33:26:33:33 | access to local variable username | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:33:26:33: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 | | 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 |