Skip to content

Commit 371c449

Browse files
committed
Add reflection-non-null-obj-for-static-member-usage.ql
1 parent c73941b commit 371c449

File tree

1 file changed

+58
-0
lines changed

1 file changed

+58
-0
lines changed
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
/**
2+
* Finds usage of the reflection classes `Field` and `Method` where they refer to
3+
* a static member, but a non-`null` value is provided as `obj` argument when the
4+
* member is used through reflection. For static members the `obj` value is ignored,
5+
* as mentioned by the documentation, therefore to avoid any confusion and to avoid
6+
* any unnecessarily verbose code, `null` should be provided as `obj` argument for
7+
* static members.
8+
*
9+
* For example:
10+
* ```java
11+
* Field f = MyClass.class.getField("MY_STATIC_FIELD");
12+
* // Argument `MyClass.class` is redundant; could simplify this to `f.get(null)`
13+
* Object value = f.get(MyClass.class);
14+
* ```
15+
*
16+
* @kind problem
17+
*/
18+
19+
import java
20+
import semmle.code.java.Reflection
21+
import semmle.code.java.dataflow.DataFlow
22+
23+
class FieldUsageMethod extends Method {
24+
FieldUsageMethod() {
25+
getDeclaringType().hasQualifiedName("java.lang.reflect", "Field")
26+
and getName().matches(["get%", "set%"])
27+
// "get%" and "set%" also match unrelated methods; verify that first parameter is `Object obj`
28+
and getParameterType(0) instanceof TypeObject
29+
}
30+
}
31+
32+
class MethodUsageMethod extends Method {
33+
MethodUsageMethod() {
34+
getDeclaringType().hasQualifiedName("java.lang.reflect", "Method")
35+
and hasName("invoke")
36+
}
37+
}
38+
39+
from ClassMethodAccess classMethodAccess, Member usedMember, MethodAccess reflectiveUsage, Expr objArgument
40+
where
41+
DataFlow::localFlow(DataFlow::exprNode(classMethodAccess), DataFlow::exprNode(reflectiveUsage.getQualifier()))
42+
// Used field or method is static
43+
and usedMember.isStatic()
44+
and objArgument = reflectiveUsage.getArgument(0)
45+
// And `obj` argument is not null
46+
and not objArgument instanceof NullLiteral
47+
and (
48+
(
49+
usedMember = classMethodAccess.(ReflectiveFieldAccess).inferAccessedField()
50+
and reflectiveUsage.getMethod() instanceof FieldUsageMethod
51+
)
52+
or (
53+
// Note: Causes some false positives due to https://github.com/github/codeql/issues/13490
54+
usedMember = classMethodAccess.(ReflectiveMethodAccess).inferAccessedMethod()
55+
and reflectiveUsage.getMethod() instanceof MethodUsageMethod
56+
)
57+
)
58+
select objArgument, "Should provide `null` as argument because member `" + usedMember.getQualifiedName() + "` accessed through reflection is static"

0 commit comments

Comments
 (0)