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 #19194 from michaelnebel/csharp/enumsimpletype

Browse Source 
C#: Extend simple type sanitizers with enums and `System.DateTimeOffset`.
This commit is contained in:
Michael Nebel
2025-04-03 10:24:26 +02:00
committed by GitHub
parent befc2fd7c1 22c943657a
commit 79688efacb
7 changed files with 79 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -756,6 +756,11 @@ class SystemDateTimeStruct extends SystemStruct {
SystemDateTimeStruct() { this.hasName("DateTime") }
}
/** The `System.DateTimeOffset` struct. */
class SystemDateTimeOffsetStruct extends SystemStruct {
SystemDateTimeOffsetStruct() { this.hasName("DateTimeOffset") }
}
/** The `System.Span<T>` struct. */
class SystemSpanStruct extends SystemUnboundGenericStruct {
SystemSpanStruct() {

4
csharp/ql/lib/semmle/code/csharp/security/Sanitizers.qll
View File

@@ -57,7 +57,9 @@ class SimpleTypeSanitizedExpr extends DataFlow::ExprNode {
SimpleTypeSanitizedExpr() {
exists(Type t | t = this.getType() or t = this.getType().(NullableType).getUnderlyingType() |
t instanceof SimpleType or
t instanceof SystemDateTimeStruct
t instanceof SystemDateTimeStruct or
t instanceof SystemDateTimeOffsetStruct or
t instanceof Enum
)
}
}

4
csharp/ql/src/change-notes/2025-04-02-simple-type-enum.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Enums and `System.DateTimeOffset` are now treated as *simple* types, which means that they are considered to have a sanitizing effect. This impacts many queries, among others the `cs/log-forging` query.

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

@@ -15,10 +15,10 @@ public class LogForgingHandler : IHttpHandler
public void ProcessRequest(HttpContext ctx)
{
String username = ctx.Request.QueryString["username"];
String username = ctx.Request.QueryString["username"]; // $ Source
ILogger logger = new ILogger();
// BAD: Logged as-is
logger.Warn(username + " logged in");
logger.Warn(username + " logged in"); // $ Alert
// GOOD: New-lines removed
logger.Warn(username.Replace(Environment.NewLine, "") + " logged in");
// GOOD: New-lines removed
@@ -28,11 +28,11 @@ public class LogForgingHandler : IHttpHandler
// GOOD: Html encoded
logger.Warn(WebUtility.HtmlEncode(username) + " logged in");
// BAD: Logged as-is to TraceSource
new TraceSource("Test").TraceInformation(username + " logged in");
new TraceSource("Test").TraceInformation(username + " logged in"); // $ Alert
Microsoft.Extensions.Logging.ILogger logger2 = null;
// BAD: Logged as-is
logger2.LogError(username);
logger2.LogError(username); // $ Alert
}
public bool IsReusable

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

@@ -2,7 +2,7 @@
| 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:31:50:31:72 | ... + ... | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:31:50:31:72 | ... + ... | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
| LogForging.cs:35:26:35:33 | access to local variable username | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:35:26:35: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:17:21:17:43 | ... + ... | LogForgingAsp.cs:13:32:13:39 | username : String | LogForgingAsp.cs:17:21:17:43 | ... + ... | This log entry depends on a $@. | LogForgingAsp.cs:13:32:13:39 | username | user-provided value |
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 | |
@@ -10,7 +10,7 @@ edges
| 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:1 |
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:18:16:18:23 | access to local variable username : String | provenance | |
| LogForgingAsp.cs:8:32:8:39 | username : String | LogForgingAsp.cs:12:21:12:43 | ... + ... | provenance | |
| LogForgingAsp.cs:13:32:13:39 | username : String | LogForgingAsp.cs:17:21:17:43 | ... + ... | provenance | |
models
| 1 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
nodes
@@ -20,6 +20,6 @@ nodes
| LogForging.cs:21:21:21:43 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:31:50:31:72 | ... + ... | semmle.label | ... + ... |
| LogForging.cs:35:26:35: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 | ... + ... |
| LogForgingAsp.cs:13:32:13:39 | username : String | semmle.label | username : String |
| LogForgingAsp.cs:17:21:17:43 | ... + ... | semmle.label | ... + ... |
subpaths

4
csharp/ql/test/query-tests/Security Features/CWE-117/LogForging.qlref
View File

@@ -1,2 +1,4 @@
query: Security Features/CWE-117/LogForging.ql
postprocess: utils/test/PrettyPrintModels.ql
postprocess:
- utils/test/PrettyPrintModels.ql
- utils/test/InlineExpectationsTestQuery.ql

58
csharp/ql/test/query-tests/Security Features/CWE-117/LogForgingAsp.cs
View File

@@ -3,13 +3,18 @@ using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Http.Headers;
using Microsoft.AspNetCore.Mvc;
public enum TestEnum
{
TestEnumValue
}
public class AspController : ControllerBase
{
public void Action1(string username)
public void Action1(string username) // $ Source
{
var logger = new ILogger();
// BAD: Logged as-is
logger.Warn(username + " logged in");
logger.Warn(username + " logged in"); // $ Alert
}
public void Action1(DateTime date)
@@ -38,4 +43,53 @@ public class AspController : ControllerBase
logger.Warn($"Warning about the bool: {b}");
}
}
public void ActionInt(int i)
{
var logger = new ILogger();
// GOOD: int is a sanitizer.
logger.Warn($"Warning about the int: {i}");
}
public void ActionLong(long l)
{
var logger = new ILogger();
// GOOD: long is a sanitizer.
logger.Warn($"Warning about the long: {l}");
}
public void ActionFloat(float f)
{
var logger = new ILogger();
// GOOD: float is a sanitizer.
logger.Warn($"Warning about the float: {f}");
}
public void ActionDouble(double d)
{
var logger = new ILogger();
// GOOD: double is a sanitizer.
logger.Warn($"Warning about the double: {d}");
}
public void ActionDecimal(decimal d)
{
var logger = new ILogger();
// GOOD: decimal is a sanitizer.
logger.Warn($"Warning about the decimal: {d}");
}
public void ActionEnum(TestEnum e)
{
var logger = new ILogger();
// GOOD: Enum is a sanitizer.
logger.Warn($"Warning about the enum: {e}");
}
public void ActionDateTime(DateTimeOffset dt)
{
var logger = new ILogger();
// GOOD: DateTimeOffset is a sanitizer.
logger.Warn($"Warning about the DateTimeOffset: {dt}");
}
}