Skip to content

Commit 98d2c84

Browse files
authored
Merge pull request #16497 from github/max-schaefer/comparison-with-wider-type
Java: Add tests for `comparison-with-wider-type`.
2 parents 5dbb91f + 3c47c11 commit 98d2c84

File tree

4 files changed

+39
-32
lines changed

4 files changed

+39
-32
lines changed

java/ql/src/Security/CWE/CWE-190/ComparisonWithWiderType.ql

Lines changed: 9 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,43 +16,20 @@
1616
import java
1717
import semmle.code.java.arithmetic.Overflow
1818

19-
int leftWidth(ComparisonExpr e) { result = e.getLeftOperand().getType().(NumType).getWidthRank() }
19+
int widthRank(Expr e) { result = e.getType().(NumType).getWidthRank() }
2020

21-
int rightWidth(ComparisonExpr e) { result = e.getRightOperand().getType().(NumType).getWidthRank() }
22-
23-
abstract class WideningComparison extends BinaryExpr instanceof ComparisonExpr {
24-
abstract Expr getNarrower();
25-
26-
abstract Expr getWider();
27-
}
28-
29-
class LTWideningComparison extends WideningComparison {
30-
LTWideningComparison() {
31-
(this instanceof LEExpr or this instanceof LTExpr) and
32-
leftWidth(this) < rightWidth(this)
33-
}
34-
35-
override Expr getNarrower() { result = this.getLeftOperand() }
36-
37-
override Expr getWider() { result = this.getRightOperand() }
38-
}
39-
40-
class GTWideningComparison extends WideningComparison {
41-
GTWideningComparison() {
42-
(this instanceof GEExpr or this instanceof GTExpr) and
43-
leftWidth(this) > rightWidth(this)
44-
}
45-
46-
override Expr getNarrower() { result = this.getRightOperand() }
47-
48-
override Expr getWider() { result = this.getLeftOperand() }
21+
predicate wideningComparison(ComparisonExpr c, Expr lesserOperand, Expr greaterOperand) {
22+
lesserOperand = c.getLesserOperand() and
23+
greaterOperand = c.getGreaterOperand() and
24+
widthRank(lesserOperand) < widthRank(greaterOperand)
4925
}
5026

51-
from WideningComparison c, LoopStmt l
27+
from ComparisonExpr c, LoopStmt l, Expr lesserOperand, Expr greaterOperand
5228
where
29+
wideningComparison(c, lesserOperand, greaterOperand) and
5330
not c.getAnOperand().isCompileTimeConstant() and
5431
l.getCondition().getAChildExpr*() = c
5532
select c,
56-
"Comparison between $@ of type " + c.getNarrower().getType().getName() + " and $@ of wider type " +
57-
c.getWider().getType().getName() + ".", c.getNarrower(), "expression", c.getWider(),
33+
"Comparison between $@ of type " + lesserOperand.getType().getName() + " and $@ of wider type " +
34+
greaterOperand.getType().getName() + ".", lesserOperand, "expression", greaterOperand,
5835
"expression"
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
| ComparisonWithWiderType.java:4:25:4:29 | ... < ... | Comparison between $@ of type int and $@ of wider type long. | ComparisonWithWiderType.java:4:25:4:25 | i | expression | ComparisonWithWiderType.java:4:29:4:29 | l | expression |
2+
| ComparisonWithWiderType.java:16:26:16:30 | ... > ... | Comparison between $@ of type byte and $@ of wider type short. | ComparisonWithWiderType.java:16:30:16:30 | b | expression | ComparisonWithWiderType.java:16:26:16:26 | c | expression |
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
public class ComparisonWithWiderType {
2+
public void testLt(long l) {
3+
// BAD: loop variable is an int, but the upper bound is a long
4+
for (int i = 0; i < l; i++) {
5+
System.out.println(i);
6+
}
7+
8+
// GOOD: loop variable is a long
9+
for (long i = 0; i < l; i++) {
10+
System.out.println(i);
11+
}
12+
}
13+
14+
public void testGt(short c) {
15+
// BAD: loop variable is a byte, but the upper bound is a short
16+
for (byte b = 0; c > b; b++) {
17+
System.out.println(b);
18+
}
19+
}
20+
21+
public void testLe(int i) {
22+
// GOOD: loop variable is a long, and the upper bound is an int
23+
for (long l = 0; l <= i; l++) {
24+
System.out.println(l);
25+
}
26+
}
27+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Security/CWE/CWE-190/ComparisonWithWiderType.ql

0 commit comments

Comments
 (0)