Skip to content

Commit 89ce25f

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#6083 from ihsinme/ihsinme-patch-275
CPP: Add query for CWE-783 Operator Precedence Logic Error When Use Bitwise Or Logical Operations
2 parents d45d588 + 6d24047 commit 89ce25f

6 files changed

+281
-0
lines changed

‎cpp/ql/src/experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
bool a=1,b=0,c=1,res;
2+
...
3+
res = a||b^c; // BAD: possible priority error `res==1`
4+
...
5+
res = a||(b^c); // GOOD: `res==1`
6+
...
7+
res = (a||b)^c; // GOOD: `res==0`

‎cpp/ql/src/experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.qhelp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Find places of confusing use of logical and bitwise operations.</p>
7+
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>We recommend using parentheses to explicitly emphasize priority.</p>
13+
14+
</recommendation>
15+
<example>
16+
<p>The following example demonstrates fallacious and fixed methods of using logical and bitwise operations.</p>
17+
<sample src="OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.c" />
18+
19+
</example>
20+
<references>
21+
22+
<li>
23+
CERT C Coding Standard:
24+
<a href="https://wiki.sei.cmu.edu/confluence/display/c/EXP00-C.+Use+parentheses+for+precedence+of+operation">EXP00-C. Use parentheses for precedence of operation</a>.
25+
</li>
26+
27+
</references>
28+
</qhelp>

‎cpp/ql/src/experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.ql

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
/**
2+
* @name Operator Precedence Logic Error When Use Bitwise Or Logical Operations
3+
* @description --Finding places to use bit and logical operations, without explicit priority allocation.
4+
* --For example, `a || b ^ c` and `(a || b) ^ c` give different results when `b` is zero.
5+
* @kind problem
6+
* @id cpp/operator-precedence-logic-error-when-use-bitwise-logical-operations
7+
* @problem.severity recommendation
8+
* @precision medium
9+
* @tags maintainability
10+
* readability
11+
* external/cwe/cwe-783
12+
* external/cwe/cwe-480
13+
*/
14+
15+
import cpp
16+
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
17+
18+
/** Holds if `exptmp` equals expression logical or followed by logical and. */
19+
predicate isLogicalOrAndExpr(LogicalOrExpr exptmp) {
20+
not exptmp.getLeftOperand() instanceof BinaryOperation and
21+
not exptmp.getRightOperand().isParenthesised() and
22+
exptmp.getRightOperand() instanceof LogicalAndExpr
23+
}
24+
25+
/** Holds if `exptmp` equals expression logical or followed by bit operation. */
26+
predicate isLogicalOrandBitwise(Expr exptmp) {
27+
not exptmp.(LogicalOrExpr).getLeftOperand() instanceof BinaryOperation and
28+
not exptmp.(LogicalOrExpr).getRightOperand().isParenthesised() and
29+
(
30+
exptmp.(LogicalOrExpr).getRightOperand().(BinaryBitwiseOperation).getLeftOperand().getType()
31+
instanceof BoolType and
32+
// The essence of these lines is to improve the quality of detection by eliminating the situation
33+
// of processing a logical type by bit operations. In fact, the predicate looks for a situation
34+
// when the left operand of a bit operation has a boolean type, which already suggests that the priority is not correct.
35+
// But if the right-hand operand is 0 or 1, then there is a possibility that the author intended so.
36+
not exptmp
37+
.(LogicalOrExpr)
38+
.getRightOperand()
39+
.(BinaryBitwiseOperation)
40+
.getRightOperand()
41+
.getValue() = "0" and
42+
not exptmp
43+
.(LogicalOrExpr)
44+
.getRightOperand()
45+
.(BinaryBitwiseOperation)
46+
.getRightOperand()
47+
.getValue() = "1"
48+
)
49+
or
50+
not exptmp.(LogicalAndExpr).getLeftOperand() instanceof BinaryOperation and
51+
not exptmp.(LogicalAndExpr).getRightOperand().isParenthesised() and
52+
(
53+
exptmp.(LogicalAndExpr).getRightOperand().(BinaryBitwiseOperation).getLeftOperand().getType()
54+
instanceof BoolType and
55+
// Looking for a situation in which the right-hand operand of a bit operation is not limited to 0 or 1.
56+
// In this case, the logical operation will be performed with the result of a binary operation that is not a Boolean type.
57+
// In my opinion this indicates a priority error. after all, it will be quite difficult for a developer
58+
// to evaluate the conversion of the results of a bit operation to a boolean type.
59+
not exptmp
60+
.(LogicalAndExpr)
61+
.getRightOperand()
62+
.(BinaryBitwiseOperation)
63+
.getRightOperand()
64+
.getValue() = "0" and
65+
not exptmp
66+
.(LogicalAndExpr)
67+
.getRightOperand()
68+
.(BinaryBitwiseOperation)
69+
.getRightOperand()
70+
.getValue() = "1"
71+
)
72+
}
73+
74+
/** Holds if `exptmp` equals expression bit operations in reverse priority order. */
75+
predicate isBitwiseandBitwise(Expr exptmp) {
76+
not exptmp.(BitwiseOrExpr).getLeftOperand() instanceof BinaryOperation and
77+
not exptmp.(BitwiseOrExpr).getRightOperand().isParenthesised() and
78+
(
79+
exptmp.(BitwiseOrExpr).getRightOperand() instanceof BitwiseAndExpr or
80+
exptmp.(BitwiseOrExpr).getRightOperand() instanceof BitwiseXorExpr
81+
)
82+
or
83+
not exptmp.(BitwiseXorExpr).getLeftOperand() instanceof BinaryOperation and
84+
not exptmp.(BitwiseXorExpr).getRightOperand().isParenthesised() and
85+
exptmp.(BitwiseXorExpr).getRightOperand() instanceof BitwiseAndExpr
86+
}
87+
88+
/** Holds if the range contains no boundary values. */
89+
predicate isRealRange(Expr exp) {
90+
upperBound(exp).toString() != "18446744073709551616" and
91+
upperBound(exp).toString() != "9223372036854775807" and
92+
upperBound(exp).toString() != "4294967295" and
93+
upperBound(exp).toString() != "Infinity" and
94+
upperBound(exp).toString() != "NaN" and
95+
lowerBound(exp).toString() != "-9223372036854775808" and
96+
lowerBound(exp).toString() != "-4294967296" and
97+
lowerBound(exp).toString() != "-Infinity" and
98+
lowerBound(exp).toString() != "NaN" and
99+
upperBound(exp) != 2147483647 and
100+
upperBound(exp) != 268435455 and
101+
upperBound(exp) != 33554431 and
102+
upperBound(exp) != 8388607 and
103+
upperBound(exp) != 65535 and
104+
upperBound(exp) != 32767 and
105+
upperBound(exp) != 255 and
106+
upperBound(exp) != 127 and
107+
lowerBound(exp) != -2147483648 and
108+
lowerBound(exp) != -268435456 and
109+
lowerBound(exp) != -33554432 and
110+
lowerBound(exp) != -8388608 and
111+
lowerBound(exp) != -65536 and
112+
lowerBound(exp) != -32768 and
113+
lowerBound(exp) != -128
114+
or
115+
lowerBound(exp) = 0 and
116+
upperBound(exp) = 1
117+
}
118+
119+
/** Holds if expressions are of different size or range */
120+
pragma[inline]
121+
predicate isDifferentSize(Expr exp1, Expr exp2, Expr exp3) {
122+
exp1.getType().getSize() = exp2.getType().getSize() and
123+
exp1.getType().getSize() != exp3.getType().getSize()
124+
or
125+
(
126+
isRealRange(exp1) and
127+
isRealRange(exp2) and
128+
isRealRange(exp3)
129+
) and
130+
upperBound(exp1).maximum(upperBound(exp2)) - upperBound(exp1).minimum(upperBound(exp2)) < 16 and
131+
lowerBound(exp1).maximum(lowerBound(exp2)) - lowerBound(exp1).minimum(lowerBound(exp2)) < 16 and
132+
(
133+
upperBound(exp1).maximum(upperBound(exp3)) - upperBound(exp1).minimum(upperBound(exp3)) > 256 or
134+
lowerBound(exp1).maximum(lowerBound(exp2)) - lowerBound(exp1).minimum(lowerBound(exp2)) > 256
135+
)
136+
}
137+
138+
/** Holds if different values of the expression obtained from the parameters of the predicate can be obtained. */
139+
pragma[inline]
140+
predicate isDifferentResults(
141+
Expr exp1, Expr exp2, Expr exp3, BinaryBitwiseOperation op1, BinaryBitwiseOperation op2
142+
) {
143+
(
144+
isRealRange(exp1) and
145+
isRealRange(exp2) and
146+
isRealRange(exp3)
147+
) and
148+
exists(int i1, int i2, int i3 |
149+
i1 in [lowerBound(exp1).floor() .. upperBound(exp1).floor()] and
150+
i2 in [lowerBound(exp2).floor() .. upperBound(exp2).floor()] and
151+
i3 in [lowerBound(exp3).floor() .. upperBound(exp3).floor()] and
152+
(
153+
op1 instanceof BitwiseOrExpr and
154+
op2 instanceof BitwiseAndExpr and
155+
i1.bitOr(i2).bitAnd(i3) != i2.bitAnd(i3).bitOr(i1)
156+
or
157+
op1 instanceof BitwiseOrExpr and
158+
op2 instanceof BitwiseXorExpr and
159+
i1.bitOr(i2).bitXor(i3) != i2.bitXor(i3).bitOr(i1)
160+
or
161+
op1 instanceof BitwiseXorExpr and
162+
op2 instanceof BitwiseAndExpr and
163+
i1.bitXor(i2).bitAnd(i3) != i2.bitAnd(i3).bitXor(i1)
164+
)
165+
)
166+
}
167+
168+
from Expr exp, string msg
169+
where
170+
isLogicalOrAndExpr(exp) and
171+
msg = "Logical AND has a higher priority."
172+
or
173+
isLogicalOrandBitwise(exp) and
174+
msg = "Binary operations have higher priority."
175+
or
176+
// Looking for a situation where the equality of the sizes of the first operands
177+
// might indicate that the developer planned to perform an operation between them.
178+
// However, the absence of parentheses means that the rightmost operation will be performed initially.
179+
isBitwiseandBitwise(exp) and
180+
isDifferentSize(exp.(BinaryBitwiseOperation).getLeftOperand(),
181+
exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getLeftOperand(),
182+
exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getRightOperand()) and
183+
msg = "Expression ranges do not match operation precedence."
184+
or
185+
// Looking for a out those expressions that, as a result of identifying the priority with parentheses,
186+
// will give different values. As a consequence, this piece of code was supposed to find errors associated
187+
// with possible outcomes of operations.
188+
isBitwiseandBitwise(exp) and
189+
isDifferentResults(exp.(BinaryBitwiseOperation).getLeftOperand(),
190+
exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getLeftOperand(),
191+
exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getRightOperand(),
192+
exp.(BinaryBitwiseOperation),
193+
exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation)) and
194+
msg = "specify the priority with parentheses."
195+
select exp, msg

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-783/semmle/tests/OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.expected

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
| test.cpp:4:6:4:15 | ... \|\| ... | Logical AND has a higher priority. |
2+
| test.cpp:11:6:11:14 | ... \|\| ... | Binary operations have higher priority. |
3+
| test.cpp:19:6:19:14 | ... \|\| ... | Binary operations have higher priority. |
4+
| test.cpp:24:6:24:13 | ... \| ... | Expression ranges do not match operation precedence. |
5+
| test.cpp:28:6:28:13 | ... ^ ... | Expression ranges do not match operation precedence. |
6+
| test.cpp:33:6:33:13 | ... \| ... | Expression ranges do not match operation precedence. |
7+
| test.cpp:38:6:38:13 | ... \| ... | specify the priority with parentheses. |

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-783/semmle/tests/OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.qlref

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
experimental/Security/CWE/CWE-783/OperatorPrecedenceLogicErrorWhenUseBitwiseOrLogicalOperations.ql

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-783/semmle/tests/test.cpp

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
void testFunction(int i1, int i2, int i3, bool b1, bool b2, bool b3, char c1)
2+
{
3+
4+
if(b1||b2&&b3) //BAD
5+
return;
6+
if((b1||b2)&&b3) //GOOD
7+
return;
8+
if(b1||(b2&&b3)) //GOOD
9+
return;
10+
11+
if(b1||b2&i1) //BAD
12+
return;
13+
if((b1||b2)&i1) //GOOD
14+
return;
15+
if(b1||b2&1) //GOOD
16+
return;
17+
if(b1&&b2&0) //GOOD
18+
return;
19+
if(b1||b2|i1) //BAD
20+
return;
21+
if((b1||b2)|i1) //GOOD
22+
return;
23+
24+
if(i1|i2&c1) //BAD
25+
return;
26+
if((i1|i2)&i3) //GOOD
27+
return;
28+
if(i1^i2&c1) //BAD
29+
return;
30+
if((i1^i2)&i3) //GOOD
31+
return;
32+
33+
if(i1|i2^c1) //BAD
34+
return;
35+
if((i1|i2)^i3) //GOOD
36+
return;
37+
38+
if(b1|b2^b3) //BAD
39+
return;
40+
if((b1|b2)^b3) //GOOD
41+
return;
42+
43+
}

0 commit comments

Comments
 (0)