|
| 1 | +/** |
| 2 | + * @name Incorrect allocation-error handling |
| 3 | + * @description `operator new` throws an exception on allocation failures, while `operator new(std::nothrow)` returns a null pointer. Mixing up these two failure conditions can result in unexpected behavior. |
| 4 | + * @kind problem |
| 5 | + * @id cpp/incorrect-allocation-error-handling |
| 6 | + * @problem.severity warning |
| 7 | + * @precision medium |
| 8 | + * @tags correctness |
| 9 | + * security |
| 10 | + * external/cwe/cwe-570 |
| 11 | + */ |
| 12 | + |
| 13 | +import cpp |
| 14 | +import semmle.code.cpp.valuenumbering.GlobalValueNumbering |
| 15 | +import semmle.code.cpp.controlflow.Guards |
| 16 | + |
| 17 | +/** |
| 18 | + * A C++ `delete` or `delete[]` expression. |
| 19 | + */ |
| 20 | +class DeleteOrDeleteArrayExpr extends Expr { |
| 21 | + DeleteOrDeleteArrayExpr() { this instanceof DeleteExpr or this instanceof DeleteArrayExpr } |
| 22 | + |
| 23 | + DeallocationFunction getDeallocator() { |
| 24 | + result = [this.(DeleteExpr).getDeallocator(), this.(DeleteArrayExpr).getDeallocator()] |
| 25 | + } |
| 26 | + |
| 27 | + Destructor getDestructor() { |
| 28 | + result = [this.(DeleteExpr).getDestructor(), this.(DeleteArrayExpr).getDestructor()] |
| 29 | + } |
| 30 | +} |
| 31 | + |
| 32 | +/** Gets the `Constructor` invoked when `newExpr` allocates memory. */ |
| 33 | +Constructor getConstructorForAllocation(NewOrNewArrayExpr newExpr) { |
| 34 | + result.getACallToThisFunction() = newExpr.getInitializer() |
| 35 | +} |
| 36 | + |
| 37 | +/** Gets the `Destructor` invoked when `deleteExpr` deallocates memory. */ |
| 38 | +Destructor getDestructorForDeallocation(DeleteOrDeleteArrayExpr deleteExpr) { |
| 39 | + result = deleteExpr.getDestructor() |
| 40 | +} |
| 41 | + |
| 42 | +/** Holds if the evaluation of `newExpr` may throw an exception. */ |
| 43 | +predicate newMayThrow(NewOrNewArrayExpr newExpr) { |
| 44 | + functionMayThrow(newExpr.getAllocator()) or |
| 45 | + functionMayThrow(getConstructorForAllocation(newExpr)) |
| 46 | +} |
| 47 | + |
| 48 | +/** Holds if the evaluation of `deleteExpr` may throw an exception. */ |
| 49 | +predicate deleteMayThrow(DeleteOrDeleteArrayExpr deleteExpr) { |
| 50 | + functionMayThrow(deleteExpr.getDeallocator()) or |
| 51 | + functionMayThrow(getDestructorForDeallocation(deleteExpr)) |
| 52 | +} |
| 53 | + |
| 54 | +/** |
| 55 | + * Holds if the function may throw an exception when called. That is, if the body of the function looks |
| 56 | + * like it might throw an exception, and the function does not have a `noexcept` or `throw()` specifier. |
| 57 | + */ |
| 58 | +predicate functionMayThrow(Function f) { |
| 59 | + (not exists(f.getBlock()) or stmtMayThrow(f.getBlock())) and |
| 60 | + not f.isNoExcept() and |
| 61 | + not f.isNoThrow() |
| 62 | +} |
| 63 | + |
| 64 | +/** Holds if the evaluation of `stmt` may throw an exception. */ |
| 65 | +predicate stmtMayThrow(Stmt stmt) { |
| 66 | + stmtMayThrow(stmt.(BlockStmt).getAStmt()) |
| 67 | + or |
| 68 | + convertedExprMayThrow(stmt.(ExprStmt).getExpr()) |
| 69 | + or |
| 70 | + convertedExprMayThrow(stmt.(DeclStmt).getADeclaration().(Variable).getInitializer().getExpr()) |
| 71 | + or |
| 72 | + exists(IfStmt ifStmt | ifStmt = stmt | |
| 73 | + convertedExprMayThrow(ifStmt.getCondition()) or |
| 74 | + stmtMayThrow([ifStmt.getThen(), ifStmt.getElse()]) |
| 75 | + ) |
| 76 | + or |
| 77 | + exists(ConstexprIfStmt constIfStmt | constIfStmt = stmt | |
| 78 | + stmtMayThrow([constIfStmt.getThen(), constIfStmt.getElse()]) |
| 79 | + ) |
| 80 | + or |
| 81 | + exists(Loop loop | loop = stmt | |
| 82 | + convertedExprMayThrow(loop.getCondition()) or |
| 83 | + stmtMayThrow(loop.getStmt()) |
| 84 | + ) |
| 85 | + or |
| 86 | + // The case for `Loop` already checked the condition and the statement. |
| 87 | + convertedExprMayThrow(stmt.(RangeBasedForStmt).getUpdate()) |
| 88 | + or |
| 89 | + // The case for `Loop` already checked the condition and the statement. |
| 90 | + exists(ForStmt forStmt | forStmt = stmt | |
| 91 | + stmtMayThrow(forStmt.getInitialization()) |
| 92 | + or |
| 93 | + convertedExprMayThrow(forStmt.getUpdate()) |
| 94 | + ) |
| 95 | + or |
| 96 | + exists(SwitchStmt switchStmt | switchStmt = stmt | |
| 97 | + convertedExprMayThrow(switchStmt.getExpr()) or |
| 98 | + stmtMayThrow(switchStmt.getStmt()) |
| 99 | + ) |
| 100 | + or |
| 101 | + // NOTE: We don't include `TryStmt` as those exceptions are not "observable" outside the function. |
| 102 | + stmtMayThrow(stmt.(Handler).getBlock()) |
| 103 | + or |
| 104 | + convertedExprMayThrow(stmt.(CoReturnStmt).getExpr()) |
| 105 | + or |
| 106 | + convertedExprMayThrow(stmt.(ReturnStmt).getExpr()) |
| 107 | +} |
| 108 | + |
| 109 | +/** Holds if the evaluation of `e` (including conversions) may throw an exception. */ |
| 110 | +predicate convertedExprMayThrow(Expr e) { |
| 111 | + exprMayThrow(e) |
| 112 | + or |
| 113 | + convertedExprMayThrow(e.getConversion()) |
| 114 | +} |
| 115 | + |
| 116 | +/** Holds if the evaluation of `e` may throw an exception. */ |
| 117 | +predicate exprMayThrow(Expr e) { |
| 118 | + e instanceof DynamicCast |
| 119 | + or |
| 120 | + e instanceof TypeidOperator |
| 121 | + or |
| 122 | + e instanceof ThrowExpr |
| 123 | + or |
| 124 | + newMayThrow(e) |
| 125 | + or |
| 126 | + deleteMayThrow(e) |
| 127 | + or |
| 128 | + convertedExprMayThrow(e.(UnaryOperation).getOperand()) |
| 129 | + or |
| 130 | + exists(BinaryOperation binOp | binOp = e | |
| 131 | + convertedExprMayThrow([binOp.getLeftOperand(), binOp.getRightOperand()]) |
| 132 | + ) |
| 133 | + or |
| 134 | + exists(Assignment assign | assign = e | |
| 135 | + convertedExprMayThrow([assign.getLValue(), assign.getRValue()]) |
| 136 | + ) |
| 137 | + or |
| 138 | + exists(CommaExpr comma | comma = e | |
| 139 | + convertedExprMayThrow([comma.getLeftOperand(), comma.getRightOperand()]) |
| 140 | + ) |
| 141 | + or |
| 142 | + exists(StmtExpr stmtExpr | stmtExpr = e | |
| 143 | + convertedExprMayThrow(stmtExpr.getResultExpr()) or |
| 144 | + stmtMayThrow(stmtExpr.getStmt()) |
| 145 | + ) |
| 146 | + or |
| 147 | + convertedExprMayThrow(e.(Conversion).getExpr()) |
| 148 | + or |
| 149 | + exists(FunctionCall fc | fc = e | |
| 150 | + not exists(fc.getTarget()) or |
| 151 | + functionMayThrow(fc.getTarget()) or |
| 152 | + convertedExprMayThrow(fc.getAnArgument()) |
| 153 | + ) |
| 154 | +} |
| 155 | + |
| 156 | +/** An allocator that might throw an exception. */ |
| 157 | +class ThrowingAllocator extends Function { |
| 158 | + ThrowingAllocator() { |
| 159 | + exists(NewOrNewArrayExpr newExpr | |
| 160 | + newExpr.getAllocator() = this and |
| 161 | + functionMayThrow(this) |
| 162 | + ) |
| 163 | + } |
| 164 | +} |
| 165 | + |
| 166 | +/** The `std::bad_alloc` exception and its `bsl` variant. */ |
| 167 | +class BadAllocType extends Class { |
| 168 | + BadAllocType() { this.hasGlobalOrStdOrBslName("bad_alloc") } |
| 169 | +} |
| 170 | + |
| 171 | +/** |
| 172 | + * A catch block that catches a `std::bad_alloc` (or any of its superclasses), or a catch |
| 173 | + * block that catches every exception (i.e., `catch(...)`). |
| 174 | + */ |
| 175 | +class BadAllocCatchBlock extends CatchBlock { |
| 176 | + BadAllocCatchBlock() { |
| 177 | + this.getParameter().getUnspecifiedType().stripType() = |
| 178 | + any(BadAllocType badAlloc).getABaseClass*() |
| 179 | + or |
| 180 | + not exists(this.getParameter()) |
| 181 | + } |
| 182 | +} |
| 183 | + |
| 184 | +/** |
| 185 | + * Holds if `newExpr` is embedded in a `try` statement with a catch block `catchBlock` that |
| 186 | + * catches a `std::bad_alloc` exception, but nothing in the `try` block (including the `newExpr`) |
| 187 | + * will throw that exception. |
| 188 | + */ |
| 189 | +predicate noThrowInTryBlock(NewOrNewArrayExpr newExpr, BadAllocCatchBlock catchBlock) { |
| 190 | + exists(TryStmt try | |
| 191 | + not stmtMayThrow(try.getStmt()) and |
| 192 | + try.getACatchClause() = catchBlock and |
| 193 | + newExpr.getEnclosingBlock().getEnclosingBlock*() = try.getStmt() |
| 194 | + ) |
| 195 | +} |
| 196 | + |
| 197 | +/** |
| 198 | + * Holds if `newExpr` is handles allocation failures by throwing an exception, yet |
| 199 | + * the guard condition `guard` compares the result of `newExpr` to a null value. |
| 200 | + */ |
| 201 | +predicate nullCheckInThrowingNew(NewOrNewArrayExpr newExpr, GuardCondition guard) { |
| 202 | + newExpr.getAllocator() instanceof ThrowingAllocator and |
| 203 | + ( |
| 204 | + // Handles null comparisons. |
| 205 | + guard.ensuresEq(globalValueNumber(newExpr).getAnExpr(), any(NullValue null), _, _, _) |
| 206 | + or |
| 207 | + // Handles `if(ptr)` and `if(!ptr)` cases. |
| 208 | + guard = globalValueNumber(newExpr).getAnExpr() |
| 209 | + ) |
| 210 | +} |
| 211 | + |
| 212 | +from NewOrNewArrayExpr newExpr, Element element, string msg, string elementString |
| 213 | +where |
| 214 | + not newExpr.isFromUninstantiatedTemplate(_) and |
| 215 | + ( |
| 216 | + noThrowInTryBlock(newExpr, element) and |
| 217 | + msg = "This allocation cannot throw. $@ is unnecessary." and |
| 218 | + elementString = "This catch block" |
| 219 | + or |
| 220 | + nullCheckInThrowingNew(newExpr, element) and |
| 221 | + msg = "This allocation cannot return null. $@ is unnecessary." and |
| 222 | + elementString = "This check" |
| 223 | + ) |
| 224 | +select newExpr, msg, element, elementString |
0 commit comments