|
| 1 | +/** |
| 2 | + * @name Redundant null check or missing null check of parameter |
| 3 | + * @description Checking a parameter for nullness in one path, |
| 4 | + * and not in another is likely to be a sign that either |
| 5 | + * the check can be removed, or added in the other case. |
| 6 | + * @kind problem |
| 7 | + * @id cpp/redundant-null-check-param |
| 8 | + * @problem.severity recommendation |
| 9 | + * @tags reliability |
| 10 | + * security |
| 11 | + * external/cwe/cwe-476 |
| 12 | + */ |
| 13 | + |
| 14 | +import cpp |
| 15 | + |
| 16 | +predicate blockDominates(Block check, Block access) { |
| 17 | + check.getLocation().getStartLine() <= access.getLocation().getStartLine() and |
| 18 | + check.getLocation().getEndLine() >= access.getLocation().getEndLine() |
| 19 | +} |
| 20 | + |
| 21 | +predicate isCheckedInstruction(VariableAccess unchecked, VariableAccess checked) { |
| 22 | + checked = any(VariableAccess va | va.getTarget() = unchecked.getTarget()) and |
| 23 | + //Simple test if the first access in this code path is dereferenced |
| 24 | + not dereferenced(checked) and |
| 25 | + blockDominates(checked.getEnclosingBlock(), unchecked.getEnclosingBlock()) |
| 26 | +} |
| 27 | + |
| 28 | +predicate candidateResultUnchecked(VariableAccess unchecked) { |
| 29 | + not isCheckedInstruction(unchecked, _) |
| 30 | +} |
| 31 | + |
| 32 | +predicate candidateResultChecked(VariableAccess check, EqualityOperation eqop) { |
| 33 | + //not dereferenced to check against pointer, not its pointed value |
| 34 | + not dereferenced(check) and |
| 35 | + //assert macros are not taken into account |
| 36 | + not check.isInMacroExpansion() and |
| 37 | + // is part of a comparison against some constant NULL |
| 38 | + eqop.getAnOperand() = check and |
| 39 | + eqop.getAnOperand() instanceof NullValue |
| 40 | +} |
| 41 | + |
| 42 | +from VariableAccess unchecked, VariableAccess check, EqualityOperation eqop, Parameter param |
| 43 | +where |
| 44 | + // a dereference |
| 45 | + dereferenced(unchecked) and |
| 46 | + // for a function parameter |
| 47 | + unchecked.getTarget() = param and |
| 48 | + // this function parameter is not overwritten |
| 49 | + count(param.getAnAssignment()) = 0 and |
| 50 | + check.getTarget() = param and |
| 51 | + // which is once checked |
| 52 | + candidateResultChecked(check, eqop) and |
| 53 | + // and which has not been checked before in this code path |
| 54 | + candidateResultUnchecked(unchecked) |
| 55 | +select check, "This null check is redundant or there is a missing null check before $@ ", unchecked, |
| 56 | + "where dereferencing happens" |
0 commit comments