Skip to content

Commit 26b7738

Browse files
committed
Add Gson type-safety queries
1 parent de2ab0d commit 26b7738

File tree

2 files changed

+146
-0
lines changed

2 files changed

+146
-0
lines changed
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
/**
2+
* Finds creation of `TypeToken` using the diamond syntax `<>`, but where the inferred
3+
* type seems to be undesired.
4+
*
5+
* For example:
6+
* ```java
7+
* // Inferred type is `TypeToken<Object>`
8+
* gson.fromJson(..., new TypeToken<>() {}.getType());
9+
* ```
10+
*
11+
* Often this issue can also be detected by using the type-safe `Gson.fromJson(..., TypeToken)`
12+
* overloads (instead of `fromJson(..., Type)`) introduced in Gson 2.10.
13+
*
14+
* @id todo
15+
* @kind problem
16+
*/
17+
18+
import java
19+
20+
// Workaround because `ClassInstanceExpr::isDiamond` does not work for anonymous
21+
// classes yet, see https://github.com/github/codeql/pull/15429
22+
predicate isDiamond(ClassInstanceExpr e) {
23+
e.isDiamond()
24+
or
25+
e.getAnonymousClass().getASupertype() instanceof ParameterizedType and
26+
not exists(e.getATypeArgument())
27+
}
28+
29+
from ClassInstanceExpr newExpr, ParameterizedType resolvedTypeTokenType
30+
where
31+
isDiamond(newExpr) and
32+
resolvedTypeTokenType = newExpr.getType().(AnonymousClass).getASupertype() and
33+
resolvedTypeTokenType
34+
.getSourceDeclaration()
35+
.hasQualifiedName("com.google.gson.reflect", "TypeToken") and
36+
resolvedTypeTokenType.getTypeArgument(0) instanceof TypeObject
37+
select newExpr, "Accidentally creates a `TypeToken<Object>`"
Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
/**
2+
* Finds usage of `Gson.fromJson(..., Type)` where the type specified through the `Type`
3+
* argument does not match the desired type used for the result. This can lead to a
4+
* `ClassCastException` at runtime, or might accidentally rely on Gson imlementation details.
5+
*
6+
* The `fromJson(..., Type)` overloads are not type-safe, the return type is not bound.
7+
* Prefer the `T fromJson(..., TypeToken<T>)` overloads which are type-safe; they were
8+
* added in Gson 2.10.
9+
*
10+
* For example:
11+
* ```java
12+
* // Bug: Type mismatch `OtherClass != MyClass`, but no compilation warning or error
13+
* List<OtherClass> l = gson.fromJson(..., new TypeToken<List<MyClass>>() {}.getType());
14+
*
15+
* // Instead prefer `fromJson(..., TypeToken)`, note the missing `.getType()` call
16+
* // This causes a compilation error, as desired, requiring to fix the incorrect types
17+
* List<OtherClass> l = gson.fromJson(..., new TypeToken<List<MyClass>>() {});
18+
* ```
19+
*
20+
* @id todo
21+
* @kind problem
22+
*/
23+
24+
import java
25+
import semmle.code.java.dataflow.DataFlow
26+
27+
/** Get `T` from `TypeToken<T>` */
28+
RefType getTypeTokenArg(RefType t) {
29+
result = t.(ParameterizedType).getTypeArgument(0)
30+
or
31+
// Or obtain it from `new TypeToken<T> () {}`, where the supertype is `TypeToken<T>`
32+
result = t.(AnonymousClass).getASupertype().(ParameterizedType).getTypeArgument(0)
33+
}
34+
35+
predicate isValidDesiredType(RefType desiredType, RefType specifiedType) {
36+
specifiedType.getASupertype*() = desiredType
37+
or
38+
// Or they are not subtypes but the type arguments of the 'desired' type are less specific
39+
// than the ones of the 'specified' type, e.g. `List<BaseClass>` vs. `List<SubClass>`
40+
// Note: This does not cover all false positives though, e.g. `List<BaseClass>` vs. `ArrayList<SubClass>`,
41+
// but that is probably rather rare and handling this would require resolving type variables
42+
exists(ParameterizedType dParamType, ParameterizedType sParamType |
43+
dParamType = desiredType and sParamType = specifiedType
44+
|
45+
dParamType.getSourceDeclaration() = sParamType.getSourceDeclaration() and
46+
forex(int typeArgIndex, RefType dTypeArg, RefType sTypeArg |
47+
dTypeArg = dParamType.getTypeArgument(typeArgIndex) and
48+
sTypeArg = sParamType.getTypeArgument(typeArgIndex)
49+
|
50+
// TODO: Maybe can make this more performant by just checking `sTypeArg.getASupertype*() = dTypeArg`,
51+
// that should suffice for most cases
52+
isValidDesiredType(dTypeArg, sTypeArg)
53+
)
54+
)
55+
}
56+
57+
from
58+
MethodAccess deserializationCall, Expr typeArg, RefType desiredType, Expr typeTokenExpr,
59+
RefType specifiedType
60+
where
61+
exists(Method calledMethod | calledMethod = deserializationCall.getMethod() |
62+
exists(Method deserializeMethod |
63+
deserializeMethod
64+
.getDeclaringType()
65+
.hasQualifiedName("com.google.gson", "JsonDeserializationContext") and
66+
deserializeMethod.hasName("deserialize") and
67+
calledMethod.getSourceDeclaration().getASourceOverriddenMethod*() = deserializeMethod and
68+
typeArg = deserializationCall.getArgument(1)
69+
)
70+
or
71+
calledMethod.getDeclaringType().hasQualifiedName("com.google.gson", "Gson") and
72+
calledMethod.hasName("fromJson") and
73+
calledMethod.getParameterType(1).(RefType).hasQualifiedName("java.lang.reflect", "Type") and
74+
typeArg = deserializationCall.getArgument(1)
75+
) and
76+
// Result type of generic method call is the desired type
77+
desiredType = deserializationCall.getType() and
78+
// Determine the `specifiedType`
79+
exists(MethodAccess getTypeCall, Method getTypeMethod |
80+
getTypeMethod = getTypeCall.getMethod() and
81+
getTypeMethod
82+
.getSourceDeclaration()
83+
.getDeclaringType()
84+
.hasQualifiedName("com.google.gson.reflect", "TypeToken") and
85+
getTypeMethod.hasName("getType") and
86+
specifiedType = getTypeTokenArg(typeTokenExpr.getType())
87+
|
88+
// Locally created `TypeToken`
89+
typeTokenExpr = getTypeCall.getQualifier() and
90+
DataFlow::localExprFlow(getTypeCall, typeArg)
91+
or
92+
exists(Field f |
93+
// Or `TypeToken` stored in field (but possibly with field type `TypeToken<?>`); try to get
94+
// type from assigned value
95+
f.getAnAssignedValue() = typeTokenExpr and
96+
getTypeCall.getQualifier() = f.getAnAccess() and
97+
DataFlow::localExprFlow(getTypeCall, typeArg)
98+
or
99+
// Or `TypeToken.getType()` stored in field
100+
f.getAnAssignedValue() = getTypeCall and
101+
typeTokenExpr = getTypeCall.getQualifier() and
102+
typeArg = f.getAnAccess()
103+
)
104+
) and
105+
// Ignore `TypeToken<?>`; can't know if `desiredType` is correct then
106+
not specifiedType instanceof Wildcard and
107+
not isValidDesiredType(desiredType, specifiedType)
108+
select deserializationCall, "Specified type '$@' differs from desired type '" + desiredType + "'",
109+
typeTokenExpr, specifiedType.toString()

0 commit comments

Comments
 (0)