diff --git a/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll b/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll index 4ca68fb27c4..e0cc92b6ce8 100644 --- a/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll +++ b/cpp/ql/src/Likely Bugs/Arithmetic/UnsignedGEZero.qll @@ -15,24 +15,33 @@ class ConstantZero extends Expr { } } +/** + * Holds if `candidate` is an expression such that if it's unsigned then we + * want an alert at `ge`. + */ +private predicate lookForUnsignedAt(GEExpr ge, Expr candidate) { + // Base case: `candidate >= 0` + ge.getRightOperand() instanceof ConstantZero and + candidate = ge.getLeftOperand().getFullyConverted() and + // left operand was a signed or unsigned IntegralType before conversions + // (not a pointer, checking a pointer >= 0 is an entirely different mistake) + // (not an enum, as the fully converted type of an enum is compiler dependent + // so checking an enum >= 0 is always reasonable) + ge.getLeftOperand().getUnderlyingType() instanceof IntegralType + or + // Recursive case: `...(largerType)candidate >= 0` + exists(Conversion conversion | + lookForUnsignedAt(ge, conversion) and + candidate = conversion.getExpr() and + conversion.getType().getSize() > candidate.getType().getSize() + ) +} + class UnsignedGEZero extends GEExpr { UnsignedGEZero() { - this.getRightOperand() instanceof ConstantZero and - // left operand was a signed or unsigned IntegralType before conversions - // (not a pointer, checking a pointer >= 0 is an entirely different mistake) - // (not an enum, as the fully converted type of an enum is compiler dependent - // so checking an enum >= 0 is always reasonable) - getLeftOperand().getUnderlyingType() instanceof IntegralType and exists(Expr ue | - // ue is some conversion of the left operand - ue = getLeftOperand().getConversion*() and - // ue is unsigned - ue.getUnderlyingType().(IntegralType).isUnsigned() and - // ue may be converted to zero or more strictly larger possibly signed types - // before it is fully converted - forall(Expr following | following = ue.getConversion+() | - following.getType().getSize() > ue.getType().getSize() - ) + lookForUnsignedAt(this, ue) and + ue.getUnderlyingType().(IntegralType).isUnsigned() ) } }