Skip to content

Commit d2df48d

Browse files
authored
Merge branch 'main' into redsun82/kotlin-language-version-min-1.9
2 parents bc59232 + 70d8c1c commit d2df48d

File tree

37 files changed

+930
-21982
lines changed

37 files changed

+930
-21982
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: feature
3+
---
4+
* Added a class `IndirectUninitializedNode` to represent the indirection of an uninitialized local variable as a dataflow node.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: feature
3+
---
4+
* Added a class `DataFlow::IndirectParameterNode` to represent the indirection of a parameter as a dataflow node.
5+
* Added a predicate `Node::asIndirectInstruction` which returns the `Instruction` that defines the indirect dataflow node, if any.

cpp/ql/lib/semmle/code/cpp/commons/Printf.qll

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -163,20 +163,31 @@ predicate primitiveVariadicFormatter(
163163
)
164164
}
165165

166+
/**
167+
* Gets a function call whose target is a variadic formatter with the given
168+
* `type`, `format` parameter index and `output` parameter index.
169+
*
170+
* Join-order helper for `callsVariadicFormatter`.
171+
*/
172+
pragma[nomagic]
173+
private predicate callsVariadicFormatterCall(FunctionCall fc, string type, int format, int output) {
174+
variadicFormatter(fc.getTarget(), type, format, output)
175+
}
176+
166177
private predicate callsVariadicFormatter(
167178
Function f, string type, int formatParamIndex, int outputParamIndex
168179
) {
169180
// calls a variadic formatter with `formatParamIndex`, `outputParamIndex` linked
170181
exists(FunctionCall fc, int format, int output |
171-
variadicFormatter(pragma[only_bind_into](fc.getTarget()), type, format, output) and
182+
callsVariadicFormatterCall(fc, type, format, output) and
172183
fc.getEnclosingFunction() = f and
173184
fc.getArgument(format) = f.getParameter(formatParamIndex).getAnAccess() and
174185
fc.getArgument(output) = f.getParameter(outputParamIndex).getAnAccess()
175186
)
176187
or
177188
// calls a variadic formatter with only `formatParamIndex` linked
178189
exists(FunctionCall fc, string calledType, int format, int output |
179-
variadicFormatter(pragma[only_bind_into](fc.getTarget()), calledType, format, output) and
190+
callsVariadicFormatterCall(fc, calledType, format, output) and
180191
fc.getEnclosingFunction() = f and
181192
fc.getArgument(format) = f.getParameter(formatParamIndex).getAnAccess() and
182193
not fc.getArgument(output) = f.getParameter(_).getAnAccess() and

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,12 @@ module Public {
321321
*/
322322
Operand asIndirectOperand(int index) { hasOperandAndIndex(this, result, index) }
323323

324+
/**
325+
* Gets the instruction that is indirectly tracked by this node behind
326+
* `index` number of indirections.
327+
*/
328+
Instruction asIndirectInstruction(int index) { hasInstructionAndIndex(this, result, index) }
329+
324330
/**
325331
* Holds if this node is at index `i` in basic block `block`.
326332
*
@@ -617,6 +623,25 @@ module Public {
617623
*/
618624
LocalVariable asUninitialized() { result = this.(UninitializedNode).getLocalVariable() }
619625

626+
/**
627+
* Gets the uninitialized local variable corresponding to this node behind
628+
* `index` number of indirections, if any.
629+
*/
630+
LocalVariable asIndirectUninitialized(int index) {
631+
exists(IndirectUninitializedNode indirectUninitializedNode |
632+
this = indirectUninitializedNode and
633+
indirectUninitializedNode.getIndirectionIndex() = index
634+
|
635+
result = indirectUninitializedNode.getLocalVariable()
636+
)
637+
}
638+
639+
/**
640+
* Gets the uninitialized local variable corresponding to this node behind
641+
* a number indirections, if any.
642+
*/
643+
LocalVariable asIndirectUninitialized() { result = this.asIndirectUninitialized(_) }
644+
620645
/**
621646
* Gets the positional parameter corresponding to the node that represents
622647
* the value of the parameter after `index` number of loads, if any. For
@@ -761,16 +786,13 @@ module Public {
761786
final override Type getType() { result = this.getPreUpdateNode().getType() }
762787
}
763788

764-
/**
765-
* The value of an uninitialized local variable, viewed as a node in a data
766-
* flow graph.
767-
*/
768-
class UninitializedNode extends Node {
789+
abstract private class AbstractUninitializedNode extends Node {
769790
LocalVariable v;
791+
int indirectionIndex;
770792

771-
UninitializedNode() {
793+
AbstractUninitializedNode() {
772794
exists(SsaImpl::Definition def, SsaImpl::SourceVariable sv |
773-
def.getIndirectionIndex() = 0 and
795+
def.getIndirectionIndex() = indirectionIndex and
774796
def.getValue().asInstruction() instanceof UninitializedInstruction and
775797
SsaImpl::defToNode(this, def, sv) and
776798
v = sv.getBaseVariable().(SsaImpl::BaseIRVariable).getIRVariable().getAst()
@@ -781,6 +803,25 @@ module Public {
781803
LocalVariable getLocalVariable() { result = v }
782804
}
783805

806+
/**
807+
* The value of an uninitialized local variable, viewed as a node in a data
808+
* flow graph.
809+
*/
810+
class UninitializedNode extends AbstractUninitializedNode {
811+
UninitializedNode() { indirectionIndex = 0 }
812+
}
813+
814+
/**
815+
* The value of an uninitialized local variable behind one or more levels of
816+
* indirection, viewed as a node in a data flow graph.
817+
*/
818+
class IndirectUninitializedNode extends AbstractUninitializedNode {
819+
IndirectUninitializedNode() { indirectionIndex > 0 }
820+
821+
/** Gets the indirection index of this node. */
822+
int getIndirectionIndex() { result = indirectionIndex }
823+
}
824+
784825
/**
785826
* The value of a parameter at function entry, viewed as a node in a data
786827
* flow graph. This includes both explicit parameters such as `x` in `f(x)`
@@ -795,6 +836,12 @@ module Public {
795836
/** An explicit positional parameter, including `this`, but not `...`. */
796837
final class DirectParameterNode = AbstractDirectParameterNode;
797838

839+
/**
840+
* A node representing an indirection of a positional parameter,
841+
* including `*this`, but not `*...`.
842+
*/
843+
final class IndirectParameterNode = AbstractIndirectParameterNode;
844+
798845
final class ExplicitParameterNode = AbstractExplicitParameterNode;
799846

800847
/** An implicit `this` parameter. */
@@ -954,11 +1001,6 @@ module Public {
9541001

9551002
private import Public
9561003

957-
/**
958-
* A node representing an indirection of a parameter.
959-
*/
960-
final class IndirectParameterNode = AbstractIndirectParameterNode;
961-
9621004
/**
9631005
* A class that lifts pre-SSA dataflow nodes to regular dataflow nodes.
9641006
*/
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* The `cs/log-forging` query no longer treats arguments to extension methods with
5+
source code on `ILogger` types as sinks. Instead, taint is tracked interprocedurally
6+
through extension method bodies, reducing false positives when extension methods
7+
sanitize input internally.

csharp/ql/lib/semmle/code/csharp/security/dataflow/LogForgingQuery.qll

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,17 @@ private class HtmlSanitizer extends Sanitizer {
5252
}
5353

5454
/**
55-
* An argument to a call to a method on a logger class.
55+
* An argument to a call to a method on a logger class, excluding extension methods
56+
* with source code which are analyzed interprocedurally.
5657
*/
57-
private class LogForgingLogMessageSink extends Sink, LogMessageSink { }
58+
private class LogForgingLogMessageSink extends Sink, LogMessageSink {
59+
LogForgingLogMessageSink() {
60+
not exists(ExtensionMethodCall mc |
61+
this.getExpr() = mc.getAnArgument() and
62+
mc.getTarget().fromSource()
63+
)
64+
}
65+
}
5866

5967
/**
6068
* An argument to a call to a method on a trace class.

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,11 @@ public void ProcessRequest(HttpContext ctx)
3333
Microsoft.Extensions.Logging.ILogger logger2 = null;
3434
// BAD: Logged as-is
3535
logger2.LogError(username); // $ Alert
36+
37+
// GOOD: uses safe extension method that sanitizes internally
38+
logger.WarnSafe(username + " logged in");
39+
// BAD: uses unsafe extension method that does not sanitize
40+
logger.WarnUnsafe(username + " logged in");
3641
}
3742

3843
public bool IsReusable
@@ -43,3 +48,16 @@ public bool IsReusable
4348
}
4449
}
4550
}
51+
52+
static class UserLoggerExtensions
53+
{
54+
public static void WarnSafe(this ILogger logger, string message)
55+
{
56+
logger.Warn(message.ReplaceLineEndings(""));
57+
}
58+
59+
public static void WarnUnsafe(this ILogger logger, string message)
60+
{
61+
logger.Warn(message); // $ Alert
62+
}
63+
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,18 @@
22
| 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 |
33
| 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 |
44
| 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 |
5+
| LogForging.cs:61:21:61:27 | access to parameter message | LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:61:21:61:27 | access to parameter message | This log entry depends on a $@. | LogForging.cs:18:27:18:49 | access to property QueryString | user-provided value |
56
| 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 |
67
edges
78
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:21:21:21:43 | ... + ... | provenance | |
89
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:31:50:31:72 | ... + ... | provenance | |
910
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:35:26:35:33 | access to local variable username | provenance | |
11+
| LogForging.cs:18:16:18:23 | access to local variable username : String | LogForging.cs:40:27:40:49 | ... + ... : String | provenance | |
1012
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:16:18:23 | access to local variable username : String | provenance | |
1113
| LogForging.cs:18:27:18:49 | access to property QueryString : NameValueCollection | LogForging.cs:18:27:18:61 | access to indexer : String | provenance | MaD:1 |
1214
| LogForging.cs:18:27:18:61 | access to indexer : String | LogForging.cs:18:16:18:23 | access to local variable username : String | provenance | |
15+
| LogForging.cs:40:27:40:49 | ... + ... : String | LogForging.cs:59:63:59:69 | message : String | provenance | |
16+
| LogForging.cs:59:63:59:69 | message : String | LogForging.cs:61:21:61:27 | access to parameter message | provenance | |
1317
| LogForgingAsp.cs:13:32:13:39 | username : String | LogForgingAsp.cs:17:21:17:43 | ... + ... | provenance | |
1418
models
1519
| 1 | Summary: System.Collections.Specialized; NameValueCollection; false; get_Item; (System.String); ; Argument[this]; ReturnValue; taint; df-generated |
@@ -20,6 +24,9 @@ nodes
2024
| LogForging.cs:21:21:21:43 | ... + ... | semmle.label | ... + ... |
2125
| LogForging.cs:31:50:31:72 | ... + ... | semmle.label | ... + ... |
2226
| LogForging.cs:35:26:35:33 | access to local variable username | semmle.label | access to local variable username |
27+
| LogForging.cs:40:27:40:49 | ... + ... : String | semmle.label | ... + ... : String |
28+
| LogForging.cs:59:63:59:69 | message : String | semmle.label | message : String |
29+
| LogForging.cs:61:21:61:27 | access to parameter message | semmle.label | access to parameter message |
2330
| LogForgingAsp.cs:13:32:13:39 | username : String | semmle.label | username : String |
2431
| LogForgingAsp.cs:17:21:17:43 | ... + ... | semmle.label | ... + ... |
2532
subpaths

docs/codeql/codeql-overview/codeql-changelog/codeql-cli-2.19.1.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ Java/Kotlin
129129
"""""""""""
130130

131131
* The Java extractor and QL libraries now support Java 23.
132-
* Kotlin versions up to 2.1.0\ *x* are now supported.
132+
* Kotlin versions up to 2.1.0*x* are now supported.
133133

134134
Python
135135
""""""

docs/codeql/codeql-overview/codeql-changelog/codeql-cli-2.21.3.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ New Features
144144
Java/Kotlin
145145
"""""""""""
146146

147-
* Kotlin versions up to 2.2.0\ *x* are now supported. Support for the Kotlin 1.5.x series is dropped (so the minimum Kotlin version is now 1.6.0).
147+
* Kotlin versions up to 2.2.0*x* are now supported. Support for the Kotlin 1.5.x series is dropped (so the minimum Kotlin version is now 1.6.0).
148148

149149
Swift
150150
"""""

0 commit comments

Comments
 (0)