hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 19:26:02 +02:00
Code Issues Packages Projects Releases Wiki Activity

Java: Add new query for large left shifts and bugfix ConstantExpAppearsNonConstant.

Browse Source
This commit is contained in:
Anders Schack-Mulligen
2020-01-23 15:33:49 +01:00
parent 8c00671f24
commit 4cb28d9b1d
12 changed files with 156 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.qhelp
View File

@@ -14,10 +14,6 @@ Some expressions always evaluate to the same result, no matter what their subexp
</li>
<li><code>x % 1</code> always evaluates to <code>0</code>.
</li>
<li><code>x &lt;&lt; 64</code> (when <code>x</code> is of type <code>long</code>) always evaluates to <code>0</code>.
</li>
<li><code>x &lt;&lt; 32</code> (when <code>x</code> is not of type <code>long</code>) always evaluates to <code>0</code>.
</li>
<li><code>x &amp; 0</code> always evaluates to <code>0</code>.
</li>
<li><code>x || true</code> always evaluates to <code>true</code>.
@@ -58,7 +54,7 @@ an integer. The correct check is <code>x % 2 == 0</code>.
<li>
The Java Language Specification:
<a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.17.1">Multiplication operator *</a>, <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.17.3">Remainder operator %</a>, <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.19">Left shift operator &lt;&lt;</a>, <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.22.1">Bitwise AND operator &amp;</a>, <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.23">Conditional-and operator &amp;&amp;</a> and <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.24">Conditional-or operator ||</a>.
<a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.17.1">Multiplication operator *</a>, <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.17.3">Remainder operator %</a>, <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.22.1">Bitwise AND operator &amp;</a>, <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.23">Conditional-and operator &amp;&amp;</a> and <a href="http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.24">Conditional-or operator ||</a>.
</li>

11
java/ql/src/Likely Bugs/Arithmetic/ConstantExpAppearsNonConstant.ql
View File

@@ -11,10 +11,6 @@
import java
int integralTypeWidth(IntegralType t) {
if t.hasName("long") or t.hasName("Long") then result = 64 else result = 32
}
int eval(Expr e) { result = e.(CompileTimeConstantExpr).getIntValue() }
predicate isConstantExp(Expr e) {
@@ -48,13 +44,6 @@ predicate isConstantExp(Expr e) {
eval(r.getRightOperand().getProperExpr()) = 1
)
or
// Left shift by 64 (longs) or 32 (all other integer types) is constant.
exists(LShiftExpr s | s = e |
exists(IntegralType t | t = s.getLeftOperand().getType() |
eval(s.getRightOperand().getProperExpr()) = integralTypeWidth(t)
)
)
or
exists(AndBitwiseExpr a | a = e | eval(a.getAnOperand().getProperExpr()) = 0)
or
exists(AndLogicalExpr a | a = e |

1
java/ql/src/Likely Bugs/Arithmetic/LShiftLargerThanTypeWidth.java Normal file
View File

@@ -0,0 +1 @@
long longVal = intVal << 32; // BAD

47
java/ql/src/Likely Bugs/Arithmetic/LShiftLargerThanTypeWidth.qhelp Normal file
View File

@@ -0,0 +1,47 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
The left shift operator only uses the lowest 5 bits of its right-hand side
when the promoted type of its left-hand side is <code>int</code> and the
lowest 6 bits when the type is <code>long</code>.
</p>
</overview>
<recommendation>
<p>
Restrict the shift amount of <code>int</code>s to the range 0-31, or cast to <code>long</code> before left-shifting.
</p>
</recommendation>
<example>
<p>
The following line tries to left-shift an <code>int</code> 32 bits.
</p>
<sample src="LShiftLargerThanTypeWidth.java" />
<p>
However, left-shifting an <code>int</code> 32 bits is equivalent to
left-shifting 0 bits, that is, not shifting at all. Instead the value should
be cast to <code>long</code> before shifting to actually left-shift 32 bits.
</p>
<sample src="LShiftLargerThanTypeWidthGood.java" />
</example>
<references>
<li>
The Java Language Specification:
<a href="https://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.19">Shift Operators</a>.
</li>
</references>
</qhelp>

26
java/ql/src/Likely Bugs/Arithmetic/LShiftLargerThanTypeWidth.ql Normal file
View File

@@ -0,0 +1,26 @@
/**
* @name Left shift by more than the type width
* @description Left-shifting an integer by more than its type width indicates a mistake.
* @kind problem
* @problem.severity warning
* @precision very-high
* @id java/lshift-larger-than-type-width
* @tags correctness
*/
import java
int integralTypeWidth(IntegralType t) {
if t.hasName("long") or t.hasName("Long") then result = 64 else result = 32
}
from LShiftExpr shift, IntegralType t, int v, string typname, int width
where
shift.getLeftOperand().getType() = t and
shift.getRightOperand().(CompileTimeConstantExpr).getIntValue() = v and
width = integralTypeWidth(t) and
v >= width and
typname = ("a " + t.toString()).regexpReplaceAll("a ([aeiouAEIOU])", "an $1")
select shift,
"Left-shifting " + typname + " by more than " + width + " truncates the shift amount from " + v +
" to " + (v % width)

1
java/ql/src/Likely Bugs/Arithmetic/LShiftLargerThanTypeWidthGood.java Normal file
View File

@@ -0,0 +1 @@
long longVal = ((long)intVal) << 32; // GOOD

17
java/ql/test/query-tests/ConstantExpAppearsNonConstant/ConstantExpAppearsNonConstant.expected
View File

@@ -5,16 +5,7 @@
| Test.java:27:28:27:44 | ... % ... | Expression always evaluates to the same value. |
| Test.java:29:29:29:48 | ... % ... | Expression always evaluates to the same value. |
| Test.java:30:32:30:50 | ... % ... | Expression always evaluates to the same value. |
| Test.java:38:32:38:59 | ... << ... | Expression always evaluates to the same value. |
| Test.java:41:32:41:59 | ... << ... | Expression always evaluates to the same value. |
| Test.java:44:33:44:61 | ... << ... | Expression always evaluates to the same value. |
| Test.java:50:31:50:49 | ... << ... | Expression always evaluates to the same value. |
| Test.java:51:33:51:52 | ... << ... | Expression always evaluates to the same value. |
| Test.java:52:35:52:55 | ... << ... | Expression always evaluates to the same value. |
| Test.java:53:38:53:57 | ... << ... | Expression always evaluates to the same value. |
| Test.java:54:37:54:58 | ... << ... | Expression always evaluates to the same value. |
| Test.java:55:40:55:61 | ... << ... | Expression always evaluates to the same value. |
| Test.java:62:30:62:46 | ... & ... | Expression always evaluates to the same value. |
| Test.java:63:31:63:47 | ... & ... | Expression always evaluates to the same value. |
| Test.java:78:32:78:58 | ... \|\| ... | Expression always evaluates to the same value. |
| Test.java:79:33:79:59 | ... \|\| ... | Expression always evaluates to the same value. |
| Test.java:37:30:37:46 | ... & ... | Expression always evaluates to the same value. |
| Test.java:38:31:38:47 | ... & ... | Expression always evaluates to the same value. |
| Test.java:53:32:53:58 | ... \|\| ... | Expression always evaluates to the same value. |
| Test.java:54:33:54:59 | ... \|\| ... | Expression always evaluates to the same value. |

25
java/ql/test/query-tests/ConstantExpAppearsNonConstant/Test.java
View File

@@ -29,31 +29,6 @@ class Test{
long rem_is_constant_hex = rnd.nextLong() % 0x1; //NOT OK
long rem_is_constant_binary = rnd.nextLong() % 01; //NOT OK
//Left shift by 32 or 64
int lshift_not_constant_int = 42 << 6; //OK
long lshift_not_constant_long = 42L << 6; //OK
int lshift_constant_cast = ((char) 42) << 16; //OK
int lshift_not_constant_byte = ((byte)rnd.nextInt(8)) << 8; //OK (not constant, because of conversion from byte to int)
int lshift_is_constant_byte = ((byte)rnd.nextInt(8)) << 32; //NOT OK
int lshift_not_constant_char = ((char)rnd.nextInt(8)) << 8; //OK (not constant, because of conversion from byte to int)
int lshift_is_constant_char = ((char)rnd.nextInt(8)) << 32; //NOT OK
int lshift_not_constant_short = ((byte)rnd.nextInt(42)) << 16; //OK (not constant, because of conversion from byte to int)
int lshift_is_constant_short = ((byte)rnd.nextInt(42)) << 32; //NOT OK
int lshift_appears_constant_int = 60 << 32; //OK
long lshift_appears_constant_long = 60L << 64; //OK
int lshift_is_not_constant = rnd.nextInt() << 2; //OK
int lshift_is_constant_int = rnd.nextInt() << 32; //NOT OK
long lshift_is_constant_long = rnd.nextLong() << 64; //NOT OK
int lshift_is_constant_int_hex = rnd.nextInt() << 0x20; //NOT OK
int lshift_is_constant_int_binary = rnd.nextInt() << 040; //NOT OK
long lshift_is_constant_long_hex = rnd.nextLong() << 0x40; //NOT OK
long lshift_is_constant_long_binary = rnd.nextLong() << 0100; //NOT OK
//Bitwise 'and' by 0
int band_not_constant = 42 & 6; //OK
int band_appears_constant_left = 0 & 60; //OK

51
java/ql/test/query-tests/LShiftLargerThanTypeWidth/A.java Normal file
View File

@@ -0,0 +1,51 @@
public class A {
void test1(byte b, char c, short s, int i, long l) {
long b1 = b << 31; // OK
long b2 = b << 32; // BAD
long b3 = b << 33; // BAD
long b4 = b << 64; // BAD
long c1 = c << 22; // OK
long c2 = c << 42; // BAD
long s1 = s << 22; // OK
long s2 = s << 42; // BAD
long i1 = i << 22; // OK
long i2 = i << 32; // BAD
long i3 = i << 42; // BAD
long i4 = i << 64; // BAD
long i5 = i << 65; // BAD
long l1 = l << 22; // OK
long l2 = l << 32; // OK
long l3 = l << 42; // OK
long l4 = l << 64; // BAD
long l5 = l << 65; // BAD
}
void test2(Byte b, Character c, Short s, Integer i, Long l) {
long b1 = b << 31; // OK
long b2 = b << 32; // BAD
long b3 = b << 33; // BAD
long b4 = b << 64; // BAD
long c1 = c << 22; // OK
long c2 = c << 42; // BAD
long s1 = s << 22; // OK
long s2 = s << 42; // BAD
long i1 = i << 22; // OK
long i2 = i << 32; // BAD
long i3 = i << 42; // BAD
long i4 = i << 64; // BAD
long i5 = i << 65; // BAD
long l1 = l << 22; // OK
long l2 = l << 32; // OK
long l3 = l << 42; // OK
long l4 = l << 64; // BAD
long l5 = l << 65; // BAD
}
}

22
java/ql/test/query-tests/LShiftLargerThanTypeWidth/LShiftLargerThanTypeWidth.expected Normal file
View File

@@ -0,0 +1,22 @@
| A.java:4:15:4:21 | ... << ... | Left-shifting a byte by more than 32 truncates the shift amount from 32 to 0 |
| A.java:5:15:5:21 | ... << ... | Left-shifting a byte by more than 32 truncates the shift amount from 33 to 1 |
| A.java:6:15:6:21 | ... << ... | Left-shifting a byte by more than 32 truncates the shift amount from 64 to 0 |
| A.java:9:15:9:21 | ... << ... | Left-shifting a char by more than 32 truncates the shift amount from 42 to 10 |
| A.java:12:15:12:21 | ... << ... | Left-shifting a short by more than 32 truncates the shift amount from 42 to 10 |
| A.java:15:15:15:21 | ... << ... | Left-shifting an int by more than 32 truncates the shift amount from 32 to 0 |
| A.java:16:15:16:21 | ... << ... | Left-shifting an int by more than 32 truncates the shift amount from 42 to 10 |
| A.java:17:15:17:21 | ... << ... | Left-shifting an int by more than 32 truncates the shift amount from 64 to 0 |
| A.java:18:15:18:21 | ... << ... | Left-shifting an int by more than 32 truncates the shift amount from 65 to 1 |
| A.java:23:15:23:21 | ... << ... | Left-shifting a long by more than 64 truncates the shift amount from 64 to 0 |
| A.java:24:15:24:21 | ... << ... | Left-shifting a long by more than 64 truncates the shift amount from 65 to 1 |
| A.java:29:15:29:21 | ... << ... | Left-shifting a Byte by more than 32 truncates the shift amount from 32 to 0 |
| A.java:30:15:30:21 | ... << ... | Left-shifting a Byte by more than 32 truncates the shift amount from 33 to 1 |
| A.java:31:15:31:21 | ... << ... | Left-shifting a Byte by more than 32 truncates the shift amount from 64 to 0 |
| A.java:34:15:34:21 | ... << ... | Left-shifting a Character by more than 32 truncates the shift amount from 42 to 10 |
| A.java:37:15:37:21 | ... << ... | Left-shifting a Short by more than 32 truncates the shift amount from 42 to 10 |
| A.java:40:15:40:21 | ... << ... | Left-shifting an Integer by more than 32 truncates the shift amount from 32 to 0 |
| A.java:41:15:41:21 | ... << ... | Left-shifting an Integer by more than 32 truncates the shift amount from 42 to 10 |
| A.java:42:15:42:21 | ... << ... | Left-shifting an Integer by more than 32 truncates the shift amount from 64 to 0 |
| A.java:43:15:43:21 | ... << ... | Left-shifting an Integer by more than 32 truncates the shift amount from 65 to 1 |
| A.java:48:15:48:21 | ... << ... | Left-shifting a Long by more than 64 truncates the shift amount from 64 to 0 |
| A.java:49:15:49:21 | ... << ... | Left-shifting a Long by more than 64 truncates the shift amount from 65 to 1 |

1
java/ql/test/query-tests/LShiftLargerThanTypeWidth/LShiftLargerThanTypeWidth.qlref Normal file
View File

@@ -0,0 +1 @@
Likely Bugs/Arithmetic/LShiftLargerThanTypeWidth.ql