|
| 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 warning |
| 8 | + * @precision medium |
| 9 | + * @tags correctness |
| 10 | + * security |
| 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(Expr exptmp) { |
| 20 | + not exptmp.(LogicalOrExpr).getLeftOperand() instanceof BinaryOperation and |
| 21 | + not exptmp.(LogicalOrExpr).getRightOperand().isParenthesised() and |
| 22 | + exptmp.(LogicalOrExpr).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 | + not exptmp |
| 33 | + .(LogicalOrExpr) |
| 34 | + .getRightOperand() |
| 35 | + .(BinaryBitwiseOperation) |
| 36 | + .getRightOperand() |
| 37 | + .getValue() = "0" and |
| 38 | + not exptmp |
| 39 | + .(LogicalOrExpr) |
| 40 | + .getRightOperand() |
| 41 | + .(BinaryBitwiseOperation) |
| 42 | + .getRightOperand() |
| 43 | + .getValue() = "1" |
| 44 | + ) |
| 45 | + or |
| 46 | + not exptmp.(LogicalAndExpr).getLeftOperand() instanceof BinaryOperation and |
| 47 | + not exptmp.(LogicalAndExpr).getRightOperand().isParenthesised() and |
| 48 | + ( |
| 49 | + exptmp.(LogicalAndExpr).getRightOperand().(BinaryBitwiseOperation).getLeftOperand().getType() |
| 50 | + instanceof BoolType and |
| 51 | + not exptmp |
| 52 | + .(LogicalAndExpr) |
| 53 | + .getRightOperand() |
| 54 | + .(BinaryBitwiseOperation) |
| 55 | + .getRightOperand() |
| 56 | + .getValue() = "0" and |
| 57 | + not exptmp |
| 58 | + .(LogicalAndExpr) |
| 59 | + .getRightOperand() |
| 60 | + .(BinaryBitwiseOperation) |
| 61 | + .getRightOperand() |
| 62 | + .getValue() = "1" |
| 63 | + ) |
| 64 | +} |
| 65 | + |
| 66 | +/** Holds if `exptmp` equals expression bit operations in reverse priority order. */ |
| 67 | +predicate isBitwiseandBitwise(Expr exptmp) { |
| 68 | + not exptmp.(BitwiseOrExpr).getLeftOperand() instanceof BinaryOperation and |
| 69 | + not exptmp.(BitwiseOrExpr).getRightOperand().isParenthesised() and |
| 70 | + ( |
| 71 | + exptmp.(BitwiseOrExpr).getRightOperand() instanceof BitwiseAndExpr or |
| 72 | + exptmp.(BitwiseOrExpr).getRightOperand() instanceof BitwiseXorExpr |
| 73 | + ) |
| 74 | + or |
| 75 | + not exptmp.(BitwiseXorExpr).getLeftOperand() instanceof BinaryOperation and |
| 76 | + not exptmp.(BitwiseXorExpr).getRightOperand().isParenthesised() and |
| 77 | + exptmp.(BitwiseXorExpr).getRightOperand() instanceof BitwiseAndExpr |
| 78 | +} |
| 79 | + |
| 80 | +/** Holds if the range contains no boundary values. */ |
| 81 | +predicate isRealRange(Expr exp) { |
| 82 | + upperBound(exp).toString() != "18446744073709551616" and |
| 83 | + upperBound(exp).toString() != "9223372036854775807" and |
| 84 | + upperBound(exp).toString() != "4294967295" and |
| 85 | + upperBound(exp).toString() != "Infinity" and |
| 86 | + upperBound(exp).toString() != "NaN" and |
| 87 | + lowerBound(exp).toString() != "-9223372036854775808" and |
| 88 | + lowerBound(exp).toString() != "-4294967296" and |
| 89 | + lowerBound(exp).toString() != "-Infinity" and |
| 90 | + lowerBound(exp).toString() != "NaN" and |
| 91 | + upperBound(exp) != 2147483647 and |
| 92 | + upperBound(exp) != 268435455 and |
| 93 | + upperBound(exp) != 33554431 and |
| 94 | + upperBound(exp) != 8388607 and |
| 95 | + upperBound(exp) != 65535 and |
| 96 | + upperBound(exp) != 32767 and |
| 97 | + upperBound(exp) != 255 and |
| 98 | + upperBound(exp) != 127 and |
| 99 | + lowerBound(exp) != -2147483648 and |
| 100 | + lowerBound(exp) != -268435456 and |
| 101 | + lowerBound(exp) != -33554432 and |
| 102 | + lowerBound(exp) != -8388608 and |
| 103 | + lowerBound(exp) != -65536 and |
| 104 | + lowerBound(exp) != -32768 and |
| 105 | + lowerBound(exp) != -128 |
| 106 | + or |
| 107 | + lowerBound(exp) = 0 and |
| 108 | + upperBound(exp) = 1 |
| 109 | +} |
| 110 | + |
| 111 | +/** Holds if expressions are of different size or range */ |
| 112 | +predicate isDifferentSize(Expr exp1, Expr exp2, Expr exp3) { |
| 113 | + exp1.getType().getSize() = exp2.getType().getSize() and |
| 114 | + exp1.getType().getSize() != exp3.getType().getSize() |
| 115 | + or |
| 116 | + ( |
| 117 | + isRealRange(exp1) and |
| 118 | + isRealRange(exp2) and |
| 119 | + isRealRange(exp3) |
| 120 | + ) and |
| 121 | + upperBound(exp1).maximum(upperBound(exp2)) - upperBound(exp1).minimum(upperBound(exp2)) < 16 and |
| 122 | + lowerBound(exp1).maximum(lowerBound(exp2)) - lowerBound(exp1).minimum(lowerBound(exp2)) < 16 and |
| 123 | + ( |
| 124 | + upperBound(exp1).maximum(upperBound(exp3)) - upperBound(exp1).minimum(upperBound(exp3)) > 256 or |
| 125 | + lowerBound(exp1).maximum(lowerBound(exp2)) - lowerBound(exp1).minimum(lowerBound(exp2)) > 256 |
| 126 | + ) |
| 127 | +} |
| 128 | + |
| 129 | +/** Holds if it is possible to get different values of the expression */ |
| 130 | +predicate isDifferentResults( |
| 131 | + Expr exp1, Expr exp2, Expr exp3, BinaryBitwiseOperation op1, BinaryBitwiseOperation op2 |
| 132 | +) { |
| 133 | + ( |
| 134 | + isRealRange(exp1) and |
| 135 | + isRealRange(exp2) and |
| 136 | + isRealRange(exp3) |
| 137 | + ) and |
| 138 | + exists(int i1, int i2, int i3 | |
| 139 | + i1 in [lowerBound(exp1).floor() .. upperBound(exp1).floor()] and |
| 140 | + i2 in [lowerBound(exp2).floor() .. upperBound(exp2).floor()] and |
| 141 | + i3 in [lowerBound(exp3).floor() .. upperBound(exp3).floor()] and |
| 142 | + ( |
| 143 | + op1 instanceof BitwiseOrExpr and |
| 144 | + op2 instanceof BitwiseAndExpr and |
| 145 | + i1.bitOr(i2).bitAnd(i3) != i2.bitAnd(i3).bitOr(i1) |
| 146 | + or |
| 147 | + op1 instanceof BitwiseOrExpr and |
| 148 | + op2 instanceof BitwiseXorExpr and |
| 149 | + i1.bitOr(i2).bitXor(i3) != i2.bitXor(i3).bitOr(i1) |
| 150 | + or |
| 151 | + op1 instanceof BitwiseXorExpr and |
| 152 | + op2 instanceof BitwiseAndExpr and |
| 153 | + i1.bitXor(i2).bitAnd(i3) != i2.bitAnd(i3).bitXor(i1) |
| 154 | + ) |
| 155 | + ) |
| 156 | +} |
| 157 | + |
| 158 | +from Expr exp, string msg |
| 159 | +where |
| 160 | + isLogicalOrAndExpr(exp) and |
| 161 | + msg = "Logical and has a higher priority." |
| 162 | + or |
| 163 | + isLogicalOrandBitwise(exp) and |
| 164 | + msg = "Binary operations has higher priority." |
| 165 | + or |
| 166 | + isBitwiseandBitwise(exp) and |
| 167 | + isDifferentSize(exp.(BinaryBitwiseOperation).getLeftOperand(), |
| 168 | + exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getLeftOperand(), |
| 169 | + exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getRightOperand()) and |
| 170 | + msg = "Expression ranges do not match operation precedence." |
| 171 | + or |
| 172 | + isBitwiseandBitwise(exp) and |
| 173 | + isDifferentResults(exp.(BinaryBitwiseOperation).getLeftOperand(), |
| 174 | + exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getLeftOperand(), |
| 175 | + exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation).getRightOperand(), |
| 176 | + exp.(BinaryBitwiseOperation), |
| 177 | + exp.(BinaryBitwiseOperation).getRightOperand().(BinaryBitwiseOperation)) and |
| 178 | + msg = "specify the priority with parentheses." |
| 179 | +select exp, msg |
0 commit comments