Skip to content

Commit feece6f

Browse files
authored
Merge branch 'github:main' into main
2 parents 5b08048 + 3d025ea commit feece6f

File tree

46 files changed

+1458
-32
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+1458
-32
lines changed

cpp/ql/lib/experimental/semmle/code/cpp/semantic/analysis/ModulusAnalysis.qll

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
* variable), and `v` is an integer in the range `[0 .. m-1]`.
55
*/
66

7+
/*
8+
* The main recursion has base cases in both `ssaModulus` (for guarded reads) and `semExprModulus`
9+
* (for constant values). The most interesting recursive case is `phiModulusRankStep`, which
10+
* handles phi inputs.
11+
*/
12+
713
private import ModulusAnalysisSpecific::Private
814
private import experimental.semmle.code.cpp.semantic.Semantic
915
private import ConstantAnalysis
@@ -162,20 +168,37 @@ private predicate phiModulusInit(SemSsaPhiNode phi, SemBound b, int val, int mod
162168
*/
163169
pragma[nomagic]
164170
private predicate phiModulusRankStep(SemSsaPhiNode phi, SemBound b, int val, int mod, int rix) {
171+
/*
172+
* base case. If any phi input is equal to `b + val` modulo `mod`, that's a potential congruence
173+
* class for the phi node.
174+
*/
175+
165176
rix = 0 and
166177
phiModulusInit(phi, b, val, mod)
167178
or
168179
exists(SemSsaVariable inp, SemSsaReadPositionPhiInputEdge edge, int v1, int m1 |
169180
mod != 1 and
170181
val = remainder(v1, mod)
171182
|
183+
/*
184+
* Recursive case. If `inp` = `b + v2` mod `m2`, we combine that with the preceding potential
185+
* congruence class `b + v1` mod `m1`. The result will be the congruence class of `v1` modulo
186+
* the greatest common denominator of `m1`, `m2`, and `v1 - v2`.
187+
*/
188+
172189
exists(int v2, int m2 |
173190
rankedPhiInput(pragma[only_bind_out](phi), inp, edge, rix) and
174191
phiModulusRankStep(phi, b, v1, m1, rix - 1) and
175192
ssaModulus(inp, edge, b, v2, m2) and
176193
mod = m1.gcd(m2).gcd(v1 - v2)
177194
)
178195
or
196+
/*
197+
* Recursive case. If `inp` = `phi` mod `m2`, we combine that with the preceding potential
198+
* congruence class `b + v1` mod `m1`. The result will be a congruence class modulo the greatest
199+
* common denominator of `m1` and `m2`.
200+
*/
201+
179202
exists(int m2 |
180203
rankedPhiInput(phi, inp, edge, rix) and
181204
phiModulusRankStep(phi, b, v1, m1, rix - 1) and
Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,16 @@
11
Framework name,URL,Package prefixes
22
Java Standard Library,,java.*
33
Java extensions,,javax.* jakarta.*
4+
Kotlin Standard Library,,kotlin*
5+
Android,,android.*
6+
Android extensions,,androidx.*
47
Apache Commons Collections,https://commons.apache.org/proper/commons-collections/,org.apache.commons.collections org.apache.commons.collections4
58
Apache Commons IO,https://commons.apache.org/proper/commons-io/,org.apache.commons.io
69
Apache Commons Lang,https://commons.apache.org/proper/commons-lang/,org.apache.commons.lang3
710
Apache Commons Text,https://commons.apache.org/proper/commons-text/,org.apache.commons.text
811
Apache HttpComponents,https://hc.apache.org/,org.apache.hc.core5.* org.apache.http
9-
Android,,android.*
12+
Apache Log4j 2,https://logging.apache.org/log4j/2.0/,org.apache.logging.log4j
1013
Google Guava,https://guava.dev/,com.google.common.*
14+
JBoss Logging,,org.jboss.logging
1115
JSON-java,https://github.com/stleary/JSON-java,org.json
1216
Spring,https://spring.io/,org.springframework.*

java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import org.jetbrains.kotlin.load.java.structure.JavaClass
2727
import org.jetbrains.kotlin.load.java.structure.JavaMethod
2828
import org.jetbrains.kotlin.load.java.structure.JavaTypeParameter
2929
import org.jetbrains.kotlin.load.java.structure.JavaTypeParameterListOwner
30+
import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass
3031
import org.jetbrains.kotlin.name.FqName
3132
import org.jetbrains.kotlin.types.Variance
3233
import org.jetbrains.kotlin.util.OperatorNameConventions
@@ -98,15 +99,29 @@ open class KotlinFileExtractor(
9899
}
99100
}
100101

102+
private fun javaBinaryDeclaresMethod(c: IrClass, name: String) =
103+
((c.source as? JavaSourceElement)?.javaElement as? BinaryJavaClass)?.methods?.any { it.name.asString() == name }
104+
105+
private fun isJavaBinaryDeclaration(f: IrFunction) =
106+
f.parentClassOrNull?.let { javaBinaryDeclaresMethod(it, f.name.asString()) } ?: false
107+
108+
private fun isJavaBinaryObjectMethodRedeclaration(d: IrDeclaration) =
109+
when (d) {
110+
is IrFunction ->
111+
when (d.name.asString()) {
112+
"toString" -> d.valueParameters.isEmpty()
113+
"hashCode" -> d.valueParameters.isEmpty()
114+
"equals" -> d.valueParameters.singleOrNull()?.type?.isNullableAny() ?: false
115+
else -> false
116+
} && isJavaBinaryDeclaration(d)
117+
else -> false
118+
}
119+
101120
@OptIn(ObsoleteDescriptorBasedAPI::class)
102121
private fun isFake(d: IrDeclarationWithVisibility): Boolean {
103-
val visibility = d.visibility
104-
if (visibility is DelegatedDescriptorVisibility && visibility.delegate == Visibilities.InvisibleFake) {
105-
return true
106-
}
107-
if (d.isFakeOverride) {
122+
val hasFakeVisibility = d.visibility.let { it is DelegatedDescriptorVisibility && it.delegate == Visibilities.InvisibleFake } || d.isFakeOverride
123+
if (hasFakeVisibility && !isJavaBinaryObjectMethodRedeclaration(d))
108124
return true
109-
}
110125
try {
111126
if ((d as? IrFunction)?.descriptor?.isHiddenToOvercomeSignatureClash == true) {
112127
return true
@@ -908,7 +923,9 @@ open class KotlinFileExtractor(
908923
else
909924
null
910925
} else {
911-
forceExtractFunction(f, parentId, extractBody, extractMethodAndParameterTypeAccesses, typeSubstitution, classTypeArgsIncludingOuterClasses).also {
926+
// Work around an apparent bug causing redeclarations of `fun toString(): String` specifically in interfaces loaded from Java classes show up like fake overrides.
927+
val overriddenVisibility = if (f.isFakeOverride && isJavaBinaryObjectMethodRedeclaration(f)) OverriddenFunctionAttributes(visibility = DescriptorVisibilities.PUBLIC) else null
928+
forceExtractFunction(f, parentId, extractBody, extractMethodAndParameterTypeAccesses, typeSubstitution, classTypeArgsIncludingOuterClasses, overriddenAttributes = overriddenVisibility).also {
912929
// The defaults-forwarder function is a static utility, not a member, so we only need to extract this for the unspecialised instance of this class.
913930
if (classTypeArgsIncludingOuterClasses.isNullOrEmpty())
914931
extractDefaultsFunction(f, parentId, extractBody, extractMethodAndParameterTypeAccesses)
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
class ConstructorWithDefaults(x: Int, y: Int = 1) { }
2+
3+
fun topLevelWithDefaults(x: Int, y: Int = 1) = 0
4+
fun String.extensionWithDefaults(x: Int, y: Int = 1) = 0
5+
6+
class LibClass {
7+
8+
fun memberWithDefaults(x: Int, y: Int = 1) = 0
9+
fun String.extensionMemberWithDefaults(x: Int, y: Int = 1) = 0
10+
11+
fun multiParameterTest(x: Int, y: Int, z: Int, w: Int = 0) = 0
12+
fun Int.multiParameterExtensionTest(x: Int, y: Int, w: Int = 0) = 0
13+
14+
}
15+
16+
class SomeToken {}
17+
18+
fun topLevelArgSource(st: SomeToken, x: Int = 0) {}
19+
fun String.extensionArgSource(st: SomeToken, x: Int = 0) {}
20+
21+
class SourceClass {
22+
23+
fun memberArgSource(st: SomeToken, x: Int = 0) {}
24+
25+
}
26+
27+
fun topLevelSink(x: Int, y: Int = 1) {}
28+
fun String.extensionSink(x: Int, y: Int = 1) {}
29+
30+
class SinkClass(x: Int, y: Int = 1) {
31+
32+
fun memberSink(x: Int, y: Int = 1) {}
33+
fun String.extensionMemberSink(x: Int, y: Int = 1) {}
34+
35+
}

java/ql/integration-tests/posix-only/kotlin/default-parameter-mad-flow/test.expected

Whitespace-only changes.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
from create_database_utils import *
2+
import subprocess
3+
4+
subprocess.check_call(["kotlinc", "lib.kt", "-d", "lib"])
5+
run_codeql_database_create(["kotlinc user.kt -cp lib"], lang="java")
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import java
2+
import semmle.code.java.dataflow.TaintTracking
3+
import TestUtilities.InlineExpectationsTest
4+
private import semmle.code.java.dataflow.ExternalFlow
5+
6+
private class Models extends SummaryModelCsv {
7+
override predicate row(string row) {
8+
row =
9+
[
10+
";ConstructorWithDefaults;true;ConstructorWithDefaults;(int,int);;Argument[0];Argument[-1];taint;manual",
11+
";LibKt;true;topLevelWithDefaults;(int,int);;Argument[0];ReturnValue;value;manual",
12+
";LibKt;true;extensionWithDefaults;(String,int,int);;Argument[1];ReturnValue;value;manual",
13+
";LibClass;true;memberWithDefaults;(int,int);;Argument[0];ReturnValue;value;manual",
14+
";LibClass;true;extensionMemberWithDefaults;(String,int,int);;Argument[1];ReturnValue;value;manual",
15+
";LibClass;true;multiParameterTest;(int,int,int,int);;Argument[0..1];ReturnValue;value;manual",
16+
";LibClass;true;multiParameterExtensionTest;(int,int,int,int);;Argument[0, 1];ReturnValue;value;manual",
17+
]
18+
}
19+
}
20+
21+
private class SourceModels extends SourceModelCsv {
22+
override predicate row(string row) {
23+
row =
24+
[
25+
";LibKt;true;topLevelArgSource;(SomeToken,int);;Argument[0];kotlinMadFlowTest;manual",
26+
";LibKt;true;extensionArgSource;(String,SomeToken,int);;Argument[1];kotlinMadFlowTest;manual",
27+
";SourceClass;true;memberArgSource;(SomeToken,int);;Argument[0];kotlinMadFlowTest;manual"
28+
]
29+
}
30+
}
31+
32+
private class SinkModels extends SinkModelCsv {
33+
override predicate row(string row) {
34+
row =
35+
[
36+
";SinkClass;true;SinkClass;(int,int);;Argument[0];kotlinMadFlowTest;manual",
37+
";LibKt;true;topLevelSink;(int,int);;Argument[0];kotlinMadFlowTest;manual",
38+
";LibKt;true;extensionSink;(String,int,int);;Argument[1];kotlinMadFlowTest;manual",
39+
";SinkClass;true;memberSink;(int,int);;Argument[0];kotlinMadFlowTest;manual",
40+
";SinkClass;true;extensionMemberSink;(String,int,int);;Argument[1];kotlinMadFlowTest;manual"
41+
]
42+
}
43+
}
44+
45+
class Config extends TaintTracking::Configuration {
46+
Config() { this = "Config" }
47+
48+
override predicate isSource(DataFlow::Node n) {
49+
n.asExpr().(MethodAccess).getCallee().getName() = "source"
50+
or
51+
sourceNode(n, "kotlinMadFlowTest")
52+
}
53+
54+
override predicate isSink(DataFlow::Node n) {
55+
n.asExpr().(Argument).getCall().getCallee().getName() = "sink"
56+
or
57+
sinkNode(n, "kotlinMadFlowTest")
58+
}
59+
}
60+
61+
class InlineFlowTest extends InlineExpectationsTest {
62+
InlineFlowTest() { this = "HasFlowTest" }
63+
64+
override string getARelevantTag() { result = "flow" }
65+
66+
override predicate hasActualResult(Location location, string element, string tag, string value) {
67+
tag = "flow" and
68+
exists(DataFlow::Node src, DataFlow::Node sink, Config c | c.hasFlow(src, sink) |
69+
sink.getLocation() = location and
70+
element = sink.toString() and
71+
value = ""
72+
)
73+
}
74+
}
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
fun source() = 1
2+
3+
fun sink(x: Any) { }
4+
5+
fun test(c: LibClass, sourcec: SourceClass, sinkc: SinkClass) {
6+
7+
sink(ConstructorWithDefaults(source(), 0)) // $ flow
8+
sink(ConstructorWithDefaults(source())) // $ flow
9+
10+
sink(topLevelWithDefaults(source(), 0)) // $ flow
11+
sink(topLevelWithDefaults(source())) // $ flow
12+
13+
sink("Hello world".extensionWithDefaults(source(), 0)) // $ flow
14+
sink("Hello world".extensionWithDefaults(source())) // $ flow
15+
16+
sink(c.memberWithDefaults(source(), 0)) // $ flow
17+
sink(c.memberWithDefaults(source())) // $ flow
18+
19+
sink(c.multiParameterTest(source(), 0, 0)) // $ flow
20+
sink(c.multiParameterTest(0, source(), 0)) // $ flow
21+
sink(c.multiParameterTest(0, 0, source()))
22+
23+
with(c) {
24+
sink("Hello world".extensionMemberWithDefaults(source(), 0)) // $ flow
25+
sink("Hello world".extensionMemberWithDefaults(source())) // $ flow
26+
}
27+
28+
with(c) {
29+
sink(source().multiParameterExtensionTest(0, 0)) // $ flow
30+
sink(0.multiParameterExtensionTest(source(), 0)) // $ flow
31+
sink(0.multiParameterExtensionTest(0, source()))
32+
}
33+
34+
run {
35+
val st = SomeToken()
36+
topLevelArgSource(st)
37+
sink(st) // $ flow
38+
}
39+
40+
run {
41+
val st = SomeToken()
42+
"Hello world".extensionArgSource(st)
43+
sink(st) // $ flow
44+
}
45+
46+
run {
47+
val st = SomeToken()
48+
sourcec.memberArgSource(st)
49+
sink(st) // $ flow
50+
}
51+
52+
SinkClass(source()) // $ flow
53+
topLevelSink(source()) // $ flow
54+
"Hello world".extensionSink(source()) // $ flow
55+
sinkc.memberSink(source()) // $ flow
56+
with(sinkc) {
57+
"Hello world".extensionMemberSink(source()) // $ flow
58+
}
59+
60+
}

java/ql/integration-tests/posix-only/kotlin/gradle_kotlinx_serialization/ConstantExpAppearsNonConstant.expected

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql

0 commit comments

Comments
 (0)