Merge remote-tracking branch 'upstream/main' into side-effects

This commit is contained in:
Dave Bartolomeo
2021-05-07 16:50:48 -04:00
465 changed files with 16260 additions and 3029 deletions

View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* The 'Unsigned difference expression compared to zero' (cpp/unsigned-difference-expression-compared-zero) query has been improved to produce fewer false positive results.

View File

@@ -0,0 +1,2 @@
codescanning
* The 'Pointer to stack object used as return value' (cpp/return-stack-allocated-object) query has been deprecated, and any uses should be replaced with `Returning stack-allocated memory` (cpp/return-stack-allocated-memory).

View File

@@ -0,0 +1,2 @@
lgtm,codescanning
* The `exprMightOverflowPositively` and `exprMightOverflowNegatively` predicates from the `SimpleRangeAnalysis` library now recognize more expressions that might overflow.

View File

@@ -39,7 +39,7 @@ then replace all the relevant occurrences in the code.</p>
</li>
<li>
Mats Henricson and Erik Nyquist, <i>Industrial Strength C++</i>, published by Prentice Hall PTR (1997).
Chapter 5: Object Life Cycle, Rec 5.4 (<a href="http://mongers.org/industrial-c++/">PDF</a>).
Chapter 5: Object Life Cycle, Rec 5.4 (<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">PDF</a>).
</li>
<li>
<a href="https://www.securecoding.cert.org/confluence/display/c/DCL06-C.+Use+meaningful+symbolic+constants+to+represent+literal+values">DCL06-C. Use meaningful symbolic constants to represent literal values</a>

View File

@@ -38,7 +38,7 @@ constant.</p>
</li>
<li>
Mats Henricson and Erik Nyquist, <i>Industrial Strength C++</i>, published by Prentice Hall PTR (1997).
Chapter 5: Object Life Cycle, Rec 5.4 (<a href="http://mongers.org/industrial-c++/">PDF</a>).
Chapter 5: Object Life Cycle, Rec 5.4 (<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">PDF</a>).
</li>
<li>
<a href="https://www.securecoding.cert.org/confluence/display/c/DCL06-C.+Use+meaningful+symbolic+constants+to+represent+literal+values">DCL06-C. Use meaningful symbolic constants to represent literal values</a>

View File

@@ -21,7 +21,7 @@ Review the purpose of the each global variable flagged by this rule and update e
<li>
Mats Henricson and Erik Nyquist, <i>Industrial Strength C++</i>, published by Prentice Hall PTR (1997).
Chapter 1: Naming, Rec 1.1 (<a href="http://mongers.org/industrial-c++/">PDF</a>).
Chapter 1: Naming, Rec 1.1 (<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">PDF</a>).
</li>
<li>
<a href="http://www.learncpp.com/cpp-tutorial/42-global-variables/">Global variables</a>.

View File

@@ -45,7 +45,7 @@ this rule.
</li>
<li>
Mats Henricson and Erik Nyquist, <i>Industrial Strength C++</i>, Rule 4.6. Prentice Hall PTR, 1997.
(<a href="http://mongers.org/industrial-c++/">PDF</a>).
(<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">PDF</a>).
</li>
<li>
cplusplus.com: <a href="http://www.cplusplus.com/doc/tutorial/control/">Control Structures</a>.

View File

@@ -7,6 +7,8 @@
* @tags reliability
* security
* external/cwe/cwe-562
* @deprecated This query is not suitable for production use and has been deprecated. Use
* cpp/return-stack-allocated-memory instead.
*/
import semmle.code.cpp.pointsto.PointsTo

View File

@@ -32,7 +32,7 @@ Check the return value of functions that return status information.
<references>
<li>
M. Henricson and E. Nyquist, <i>Industrial Strength C++</i>, Chapter 12: Error handling. Prentice Hall PTR, 1997 (<a href="http://mongers.org/industrial-c++/">available online</a>).
M. Henricson and E. Nyquist, <i>Industrial Strength C++</i>, Chapter 12: Error handling. Prentice Hall PTR, 1997 (<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">available online</a>).
</li>
<li>
The CERT C Secure Coding Standard: <a href="https://www.securecoding.cert.org/confluence/display/perl/EXP32-PL.+Do+not+ignore+function+return+values">EXP32-PL. Do not ignore function return values</a>.

View File

@@ -9,6 +9,8 @@
* @id cpp/signed-overflow-check
* @tags correctness
* security
* external/cwe/cwe-128
* external/cwe/cwe-190
*/
import cpp

View File

@@ -7,12 +7,12 @@
* @kind path-problem
* @problem.severity warning
* @precision high
* @id cpp/upcast-array-pointer-arithmetic
* @tags correctness
* reliability
* security
* external/cwe/cwe-119
* external/cwe/cwe-843
* @id cpp/upcast-array-pointer-arithmetic
*/
import cpp

View File

@@ -8,6 +8,8 @@
* @tags reliability
* correctness
* security
* external/cwe/cwe-190
* external/cwe/cwe-253
*/
import cpp

View File

@@ -9,6 +9,7 @@
* @tags reliability
* correctness
* security
* external/cwe/cwe-234
* external/cwe/cwe-685
*/

View File

@@ -26,7 +26,7 @@ indication that there may be cases unhandled by the <code>switch</code> statemen
MSDN Library: <a href="https://docs.microsoft.com/en-us/cpp/cpp/switch-statement-cpp">switch statement (C++)</a>
</li>
<li>
M. Henricson and E. Nyquist, <i>Industrial Strength C++</i>, Chapter 4: Control Flow, Rec 4.5. Prentice Hall PTR, 1997 (<a href="http://mongers.org/industrial-c++/">available online</a>).
M. Henricson and E. Nyquist, <i>Industrial Strength C++</i>, Chapter 4: Control Flow, Rec 4.5. Prentice Hall PTR, 1997 (<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">available online</a>).
</li>

View File

@@ -8,6 +8,7 @@
* @id cpp/pointer-overflow-check
* @tags reliability
* security
* external/cwe/cwe-758
*/
import cpp

View File

@@ -10,6 +10,7 @@
* @tags correctness
* language-features
* security
* external/cwe/cwe-670
*/
import cpp

View File

@@ -12,6 +12,8 @@
* @tags correctness
* maintainability
* security
* external/cwe/cwe-234
* external/cwe/cwe-685
*/
import cpp

View File

@@ -29,7 +29,7 @@ build time: the more included files, the longer the compilation time.</p>
<a href="http://www.drdobbs.com/cpp/decoupling-c-header-files/212701130">Decoupling C Header Files</a>
</li>
<li>
<a href="https://wiki.hsr.ch/Prog3/files/overload72-FINAL_DesigningHeaderFiles.pdf">C++ Best Practice -
<a href="https://accu.org/journals/overload/14/72/griffiths_1995/">C++ Best Practice -
Designing Header Files</a>
</li>

View File

@@ -35,7 +35,7 @@ they are contributing to unnecessarily long build times and creating artificial
<a href="http://www.drdobbs.com/cpp/decoupling-c-header-files/212701130">Decoupling C Header Files</a>
</li>
<li>
<a href="https://wiki.hsr.ch/Prog3/files/overload72-FINAL_DesigningHeaderFiles.pdf">C++ Best Practice -
<a href="https://accu.org/journals/overload/14/72/griffiths_1995/">C++ Best Practice -
Designing Header Files</a>
</li>
</references>

View File

@@ -28,6 +28,7 @@ predicate outOfBoundsExpr(Expr expr, string kind) {
from Expr use, Expr origin, string kind
where
not use.getUnspecifiedType() instanceof PointerType and
outOfBoundsExpr(use, kind) and
tainted(origin, use) and
origin != use and

View File

@@ -12,29 +12,60 @@
import cpp
import semmle.code.cpp.commons.Exclusions
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
import semmle.code.cpp.rangeanalysis.RangeAnalysisUtils
import semmle.code.cpp.controlflow.Guards
import semmle.code.cpp.dataflow.DataFlow
/** Holds if `sub` is guarded by a condition which ensures that `left >= right`. */
/**
* Holds if `sub` is guarded by a condition which ensures that
* `left >= right`.
*/
pragma[noinline]
predicate isGuarded(SubExpr sub, Expr left, Expr right) {
exists(GuardCondition guard |
guard.controls(sub.getBasicBlock(), true) and
guard.ensuresLt(left, right, 0, sub.getBasicBlock(), false)
exists(GuardCondition guard, int k |
guard.controls(sub.getBasicBlock(), _) and
guard.ensuresLt(left, right, k, sub.getBasicBlock(), false) and
k >= 0
)
}
/** Holds if `sub` will never be negative. */
predicate nonNegative(SubExpr sub) {
not exprMightOverflowNegatively(sub.getFullyConverted())
/**
* Holds if `n` is known or suspected to be less than or equal to
* `sub.getLeftOperand()`.
*/
predicate exprIsSubLeftOrLess(SubExpr sub, DataFlow::Node n) {
n.asExpr() = sub.getLeftOperand()
or
// The subtraction is guarded by a check of the form `left >= right`.
exists(GVN left, GVN right |
// This is basically a poor man's version of a directional unbind operator.
strictcount([left, globalValueNumber(sub.getLeftOperand())]) = 1 and
strictcount([right, globalValueNumber(sub.getRightOperand())]) = 1 and
isGuarded(sub, left.getAnExpr(), right.getAnExpr())
exists(DataFlow::Node other |
// dataflow
exprIsSubLeftOrLess(sub, other) and
(
DataFlow::localFlowStep(n, other) or
DataFlow::localFlowStep(other, n)
)
)
or
exists(DataFlow::Node other |
// guard constraining `sub`
exprIsSubLeftOrLess(sub, other) and
isGuarded(sub, other.asExpr(), n.asExpr()) // other >= n
)
or
exists(DataFlow::Node other, float p, float q |
// linear access of `other`
exprIsSubLeftOrLess(sub, other) and
linearAccess(n.asExpr(), other.asExpr(), p, q) and // n = p * other + q
p <= 1 and
q <= 0
)
or
exists(DataFlow::Node other, float p, float q |
// linear access of `n`
exprIsSubLeftOrLess(sub, other) and
linearAccess(other.asExpr(), n.asExpr(), p, q) and // other = p * n + q
p >= 1 and
q >= 0
)
}
@@ -45,5 +76,6 @@ where
ro.getLesserOperand().getValue().toInt() = 0 and
ro.getGreaterOperand() = sub and
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
not nonNegative(sub)
exprMightOverflowNegatively(sub.getFullyConverted()) and // generally catches false positives involving constants
not exprIsSubLeftOrLess(sub, DataFlow::exprNode(sub.getRightOperand())) // generally catches false positives where there's a relation between the left and right operands
select ro, "Unsigned subtraction can never be negative."

View File

@@ -0,0 +1,43 @@
// BAD: the allocation will throw an unhandled exception
// instead of returning a null pointer.
void bad1(std::size_t length) noexcept {
int* dest = new int[length];
if(!dest) {
return;
}
std::memset(dest, 0, length);
// ...
}
// BAD: the allocation won't throw an exception, but
// instead return a null pointer.
void bad2(std::size_t length) noexcept {
try {
int* dest = new(std::nothrow) int[length];
std::memset(dest, 0, length);
// ...
} catch(std::bad_alloc&) {
// ...
}
}
// GOOD: the allocation failure is handled appropiately.
void good1(std::size_t length) noexcept {
try {
int* dest = new int[length];
std::memset(dest, 0, length);
// ...
} catch(std::bad_alloc&) {
// ...
}
}
// GOOD: the allocation failure is handled appropiately.
void good2(std::size_t length) noexcept {
int* dest = new int[length];
if(!dest) {
return;
}
std::memset(dest, 0, length);
// ...
}

View File

@@ -0,0 +1,31 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>Different overloads of the <code>new</code> operator handle allocation failures in different ways.
If <code>new T</code> fails for some type <code>T</code>, it throws a <code>std::bad_alloc</code> exception,
but <code>new(std::nothrow) T</code> returns a null pointer. If the programmer does not use the corresponding
method of error handling, allocation failure may go unhandled and could cause the program to behave in
unexpected ways.</p>
</overview>
<recommendation>
<p>Make sure that exceptions are handled appropriately if <code>new T</code> is used. On the other hand,
make sure to handle the possibility of null pointers if <code>new(std::nothrow) T</code> is used.</p>
</recommendation>
<example>
<sample src="IncorrectAllocationErrorHandling.cpp" />
</example>
<references>
<li>
CERT C++ Coding Standard:
<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM52-CPP.+Detect+and+handle+memory+allocation+errors">MEM52-CPP. Detect and handle memory allocation errors</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,224 @@
/**
* @name Incorrect allocation-error handling
* @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.
* @kind problem
* @id cpp/incorrect-allocation-error-handling
* @problem.severity warning
* @precision medium
* @tags correctness
* security
* external/cwe/cwe-570
*/
import cpp
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.controlflow.Guards
/**
* A C++ `delete` or `delete[]` expression.
*/
class DeleteOrDeleteArrayExpr extends Expr {
DeleteOrDeleteArrayExpr() { this instanceof DeleteExpr or this instanceof DeleteArrayExpr }
DeallocationFunction getDeallocator() {
result = [this.(DeleteExpr).getDeallocator(), this.(DeleteArrayExpr).getDeallocator()]
}
Destructor getDestructor() {
result = [this.(DeleteExpr).getDestructor(), this.(DeleteArrayExpr).getDestructor()]
}
}
/** Gets the `Constructor` invoked when `newExpr` allocates memory. */
Constructor getConstructorForAllocation(NewOrNewArrayExpr newExpr) {
result.getACallToThisFunction() = newExpr.getInitializer()
}
/** Gets the `Destructor` invoked when `deleteExpr` deallocates memory. */
Destructor getDestructorForDeallocation(DeleteOrDeleteArrayExpr deleteExpr) {
result = deleteExpr.getDestructor()
}
/** Holds if the evaluation of `newExpr` may throw an exception. */
predicate newMayThrow(NewOrNewArrayExpr newExpr) {
functionMayThrow(newExpr.getAllocator()) or
functionMayThrow(getConstructorForAllocation(newExpr))
}
/** Holds if the evaluation of `deleteExpr` may throw an exception. */
predicate deleteMayThrow(DeleteOrDeleteArrayExpr deleteExpr) {
functionMayThrow(deleteExpr.getDeallocator()) or
functionMayThrow(getDestructorForDeallocation(deleteExpr))
}
/**
* Holds if the function may throw an exception when called. That is, if the body of the function looks
* like it might throw an exception, and the function does not have a `noexcept` or `throw()` specifier.
*/
predicate functionMayThrow(Function f) {
(not exists(f.getBlock()) or stmtMayThrow(f.getBlock())) and
not f.isNoExcept() and
not f.isNoThrow()
}
/** Holds if the evaluation of `stmt` may throw an exception. */
predicate stmtMayThrow(Stmt stmt) {
stmtMayThrow(stmt.(BlockStmt).getAStmt())
or
convertedExprMayThrow(stmt.(ExprStmt).getExpr())
or
convertedExprMayThrow(stmt.(DeclStmt).getADeclaration().(Variable).getInitializer().getExpr())
or
exists(IfStmt ifStmt | ifStmt = stmt |
convertedExprMayThrow(ifStmt.getCondition()) or
stmtMayThrow([ifStmt.getThen(), ifStmt.getElse()])
)
or
exists(ConstexprIfStmt constIfStmt | constIfStmt = stmt |
stmtMayThrow([constIfStmt.getThen(), constIfStmt.getElse()])
)
or
exists(Loop loop | loop = stmt |
convertedExprMayThrow(loop.getCondition()) or
stmtMayThrow(loop.getStmt())
)
or
// The case for `Loop` already checked the condition and the statement.
convertedExprMayThrow(stmt.(RangeBasedForStmt).getUpdate())
or
// The case for `Loop` already checked the condition and the statement.
exists(ForStmt forStmt | forStmt = stmt |
stmtMayThrow(forStmt.getInitialization())
or
convertedExprMayThrow(forStmt.getUpdate())
)
or
exists(SwitchStmt switchStmt | switchStmt = stmt |
convertedExprMayThrow(switchStmt.getExpr()) or
stmtMayThrow(switchStmt.getStmt())
)
or
// NOTE: We don't include `TryStmt` as those exceptions are not "observable" outside the function.
stmtMayThrow(stmt.(Handler).getBlock())
or
convertedExprMayThrow(stmt.(CoReturnStmt).getExpr())
or
convertedExprMayThrow(stmt.(ReturnStmt).getExpr())
}
/** Holds if the evaluation of `e` (including conversions) may throw an exception. */
predicate convertedExprMayThrow(Expr e) {
exprMayThrow(e)
or
convertedExprMayThrow(e.getConversion())
}
/** Holds if the evaluation of `e` may throw an exception. */
predicate exprMayThrow(Expr e) {
e instanceof DynamicCast
or
e instanceof TypeidOperator
or
e instanceof ThrowExpr
or
newMayThrow(e)
or
deleteMayThrow(e)
or
convertedExprMayThrow(e.(UnaryOperation).getOperand())
or
exists(BinaryOperation binOp | binOp = e |
convertedExprMayThrow([binOp.getLeftOperand(), binOp.getRightOperand()])
)
or
exists(Assignment assign | assign = e |
convertedExprMayThrow([assign.getLValue(), assign.getRValue()])
)
or
exists(CommaExpr comma | comma = e |
convertedExprMayThrow([comma.getLeftOperand(), comma.getRightOperand()])
)
or
exists(StmtExpr stmtExpr | stmtExpr = e |
convertedExprMayThrow(stmtExpr.getResultExpr()) or
stmtMayThrow(stmtExpr.getStmt())
)
or
convertedExprMayThrow(e.(Conversion).getExpr())
or
exists(FunctionCall fc | fc = e |
not exists(fc.getTarget()) or
functionMayThrow(fc.getTarget()) or
convertedExprMayThrow(fc.getAnArgument())
)
}
/** An allocator that might throw an exception. */
class ThrowingAllocator extends Function {
ThrowingAllocator() {
exists(NewOrNewArrayExpr newExpr |
newExpr.getAllocator() = this and
functionMayThrow(this)
)
}
}
/** The `std::bad_alloc` exception and its `bsl` variant. */
class BadAllocType extends Class {
BadAllocType() { this.hasGlobalOrStdOrBslName("bad_alloc") }
}
/**
* A catch block that catches a `std::bad_alloc` (or any of its superclasses), or a catch
* block that catches every exception (i.e., `catch(...)`).
*/
class BadAllocCatchBlock extends CatchBlock {
BadAllocCatchBlock() {
this.getParameter().getUnspecifiedType().stripType() =
any(BadAllocType badAlloc).getABaseClass*()
or
not exists(this.getParameter())
}
}
/**
* Holds if `newExpr` is embedded in a `try` statement with a catch block `catchBlock` that
* catches a `std::bad_alloc` exception, but nothing in the `try` block (including the `newExpr`)
* will throw that exception.
*/
predicate noThrowInTryBlock(NewOrNewArrayExpr newExpr, BadAllocCatchBlock catchBlock) {
exists(TryStmt try |
not stmtMayThrow(try.getStmt()) and
try.getACatchClause() = catchBlock and
newExpr.getEnclosingBlock().getEnclosingBlock*() = try.getStmt()
)
}
/**
* Holds if `newExpr` is handles allocation failures by throwing an exception, yet
* the guard condition `guard` compares the result of `newExpr` to a null value.
*/
predicate nullCheckInThrowingNew(NewOrNewArrayExpr newExpr, GuardCondition guard) {
newExpr.getAllocator() instanceof ThrowingAllocator and
(
// Handles null comparisons.
guard.ensuresEq(globalValueNumber(newExpr).getAnExpr(), any(NullValue null), _, _, _)
or
// Handles `if(ptr)` and `if(!ptr)` cases.
guard = globalValueNumber(newExpr).getAnExpr()
)
}
from NewOrNewArrayExpr newExpr, Element element, string msg, string elementString
where
not newExpr.isFromUninstantiatedTemplate(_) and
(
noThrowInTryBlock(newExpr, element) and
msg = "This allocation cannot throw. $@ is unnecessary." and
elementString = "This catch block"
or
nullCheckInThrowingNew(newExpr, element) and
msg = "This allocation cannot return null. $@ is unnecessary." and
elementString = "This check"
)
select newExpr, msg, element, elementString

View File

@@ -1,35 +0,0 @@
// BAD: on memory allocation error, the program terminates.
void badFunction(const int *source, std::size_t length) noexcept {
int * dest = new int[length];
std::memset(dest, 0, length);
// ..
}
// GOOD: memory allocation error will be handled.
void goodFunction(const int *source, std::size_t length) noexcept {
try {
int * dest = new int[length];
} catch(std::bad_alloc) {
// ...
}
std::memset(dest, 0, length);
// ..
}
// BAD: memory allocation error will not be handled.
void badFunction(const int *source, std::size_t length) noexcept {
try {
int * dest = new (std::nothrow) int[length];
} catch(std::bad_alloc) {
// ...
}
std::memset(dest, 0, length);
// ..
}
// GOOD: memory allocation error will be handled.
void goodFunction(const int *source, std::size_t length) noexcept {
int * dest = new (std::nothrow) int[length];
if (!dest) {
return;
}
std::memset(dest, 0, length);
// ..
}

View File

@@ -1,27 +0,0 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>When using the <code>new</code> operator to allocate memory, you need to pay attention to the different ways of detecting errors. <code>::operator new(std::size_t)</code> throws an exception on error, whereas <code>::operator new(std::size_t, const std::nothrow_t &amp;)</code> returns zero on error. The programmer can get confused and check the error that occurs when allocating memory incorrectly. That can lead to an unhandled program termination or to a violation of the program logic.</p>
</overview>
<recommendation>
<p>Use the correct error detection method corresponding with the memory allocation.</p>
</recommendation>
<example>
<p>The following example demonstrates various approaches to detecting memory allocation errors using the <code>new</code> operator.</p>
<sample src="WrongInDetectingAndHandlingMemoryAllocationErrors.cpp" />
</example>
<references>
<li>
CERT C++ Coding Standard:
<a href="https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM52-CPP.+Detect+and+handle+memory+allocation+errors">MEM52-CPP. Detect and handle memory allocation errors</a>.
</li>
</references>
</qhelp>

View File

@@ -1,87 +0,0 @@
/**
* @name Detect And Handle Memory Allocation Errors
* @description --::operator new(std::size_t) throws an exception on error, and ::operator new(std::size_t, const std::nothrow_t &) returns zero on error.
* --the programmer can get confused when check the error that occurs when allocating memory incorrectly.
* @kind problem
* @id cpp/detect-and-handle-memory-allocation-errors
* @problem.severity warning
* @precision medium
* @tags correctness
* security
* external/cwe/cwe-570
*/
import cpp
/**
* Lookup if condition compare with 0
*/
class IfCompareWithZero extends IfStmt {
IfCompareWithZero() {
this.getCondition().(EQExpr).getAChild().getValue() = "0"
or
this.getCondition().(NEExpr).getAChild().getValue() = "0" and
this.hasElse()
or
this.getCondition().(NEExpr).getAChild().getValue() = "0" and
this.getThen().getAChild*() instanceof ReturnStmt
}
}
/**
* lookup for calls to `operator new`, with incorrect error handling.
*/
class WrongCheckErrorOperatorNew extends FunctionCall {
Expr exp;
WrongCheckErrorOperatorNew() {
this = exp.(NewOrNewArrayExpr).getAChild().(FunctionCall) and
(
this.getTarget().hasGlobalOrStdName("operator new")
or
this.getTarget().hasGlobalOrStdName("operator new[]")
)
}
/**
* Holds if handler `try ... catch` exists.
*/
predicate isExistsTryCatchBlock() {
exists(TryStmt ts | this.getEnclosingStmt() = ts.getStmt().getAChild*())
}
/**
* Holds if results call `operator new` check in `operator if`.
*/
predicate isExistsIfCondition() {
exists(IfCompareWithZero ifc, AssignExpr aex, Initializer it |
// call `operator new` directly from the condition of `operator if`.
this = ifc.getCondition().getAChild*()
or
// check results call `operator new` with variable appropriation
postDominates(ifc, this) and
aex.getAChild() = exp and
ifc.getCondition().getAChild().(VariableAccess).getTarget() =
aex.getLValue().(VariableAccess).getTarget()
or
// check results call `operator new` with declaration variable
postDominates(ifc, this) and
exp = it.getExpr() and
it.getDeclaration() = ifc.getCondition().getAChild().(VariableAccess).getTarget()
)
}
/**
* Holds if `(std::nothrow)` or `(std::noexcept)` exists in call `operator new`.
*/
predicate isExistsNothrow() { getTarget().isNoExcept() or getTarget().isNoThrow() }
}
from WrongCheckErrorOperatorNew op
where
// use call `operator new` with `(std::nothrow)` and checking error using `try ... catch` block and not `operator if`
op.isExistsNothrow() and not op.isExistsIfCondition() and op.isExistsTryCatchBlock()
or
// use call `operator new` without `(std::nothrow)` and checking error using `operator if` and not `try ... catch` block
not op.isExistsNothrow() and not op.isExistsTryCatchBlock() and op.isExistsIfCondition()
select op, "memory allocation error check is incorrect or missing"

View File

@@ -0,0 +1,17 @@
while(flagsLoop)
{
...
if(flagsIf) break;
...
}while(flagsLoop); // BAD: when exiting through `break`, it is possible to get into an eternal loop.
...
while(flagsLoop)
{
...
if(flagsIf) break;
...
} // GOOD: correct cycle
...
if(intA+intB) return 1; // BAD: possibly no comparison
...
if(intA+intB>intC) return 1; // GOOD: correct comparison

View File

@@ -0,0 +1,28 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>In some situations, after code refactoring, parts of the old constructs may remain. They are correctly accepted by the compiler, but can critically affect program execution. For example, if you switch from `do {...} while ();` to `while () {...}` forgetting to remove the old construct completely, you get `while(){...}while();` which may be vulnerable. These code snippets look suspicious and require the developer's attention.</p>
</overview>
<recommendation>
<p>We recommend that you use more explicit code transformations.</p>
</recommendation>
<example>
<p>The following example demonstrates the erroneous and corrected sections of the code.</p>
<sample src="InsufficientControlFlowManagementAfterRefactoringTheCode.c" />
</example>
<references>
<li>
CWE Common Weakness Enumeration:
<a href="https://cwe.mitre.org/data/definitions/691.html"> CWE-691: Insufficient Control Flow Management</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,119 @@
/**
* @name Errors After Refactoring
* @description --In some situations, after code refactoring, parts of the old constructs may remain.
* --They are correctly accepted by the compiler, but can critically affect program execution.
* --For example, if you switch from `do {...} while ();` to `while () {...}` with errors, you run the risk of running out of resources.
* --These code snippets look suspicious and require the developer's attention.
* @kind problem
* @id cpp/errors-after-refactoring
* @problem.severity warning
* @precision medium
* @tags correctness
* security
* external/cwe/cwe-691
*/
import cpp
import semmle.code.cpp.valuenumbering.HashCons
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
/**
* Using `while` directly after the body of another` while`.
*/
class UsingWhileAfterWhile extends WhileStmt {
/**
* Using a loop call after another loop has finished running can result in an eternal loop.
* For example, perhaps as a result of refactoring, the `do ... while ()` loop was incorrectly corrected.
* Even in the case of deliberate use of such an expression, it is better to correct it.
*/
UsingWhileAfterWhile() {
exists(WhileStmt wh1 |
wh1.getStmt().getAChild*().(BreakStmt).(ControlFlowNode).getASuccessor().getASuccessor() =
this and
hashCons(wh1.getCondition()) = hashCons(this.getCondition()) and
this.getStmt() instanceof EmptyStmt
)
or
exists(ForStmt fr1 |
fr1.getStmt().getAChild*().(BreakStmt).(ControlFlowNode).getASuccessor().getASuccessor() =
this and
hashCons(fr1.getCondition()) = hashCons(this.getCondition()) and
this.getStmt() instanceof EmptyStmt
)
}
}
/**
* Using arithmetic in a condition.
*/
class UsingArithmeticInComparison extends BinaryArithmeticOperation {
/**
* Using arithmetic operations in a comparison operation can be dangerous.
* For example, part of the comparison may have been lost as a result of refactoring.
* Even if you deliberately use such an expression, it is better to add an explicit comparison.
*/
UsingArithmeticInComparison() {
this.getParent*() instanceof IfStmt and
not this.getAChild*().isConstant() and
not this.getParent*() instanceof Call and
not this.getParent*() instanceof AssignExpr and
not this.getParent*() instanceof ArrayExpr and
not this.getParent*() instanceof RemExpr and
not this.getParent*() instanceof AssignBitwiseOperation and
not this.getParent*() instanceof AssignArithmeticOperation and
not this.getParent*() instanceof EqualityOperation and
not this.getParent*() instanceof RelationalOperation
}
/** Holds when the expression is inside the loop body. */
predicate insideTheLoop() { exists(Loop lp | lp.getStmt().getAChild*() = this.getParent*()) }
/** Holds when the expression is used in binary operations. */
predicate workingWithValue() {
this.getParent*() instanceof BinaryBitwiseOperation or
this.getParent*() instanceof NotExpr
}
/** Holds when the expression contains a pointer. */
predicate workingWithPointer() {
this.getAChild*().getFullyConverted().getType() instanceof DerivedType
}
/** Holds when a null comparison expression exists. */
predicate compareWithZero() {
exists(Expr exp |
exp instanceof ComparisonOperation and
(
globalValueNumber(exp.getAChild*()) = globalValueNumber(this) or
hashCons(exp.getAChild*()) = hashCons(this)
) and
(
exp.(ComparisonOperation).getLeftOperand().getValue() = "0" or
exp.(ComparisonOperation).getRightOperand().getValue() = "0"
)
)
}
/** Holds when a comparison expression exists. */
predicate compareWithOutZero() {
exists(Expr exp |
exp instanceof ComparisonOperation and
(
globalValueNumber(exp.getAChild*()) = globalValueNumber(this) or
hashCons(exp.getAChild*()) = hashCons(this)
)
)
}
}
from Expr exp
where
exp instanceof UsingArithmeticInComparison and
not exp.(UsingArithmeticInComparison).workingWithValue() and
not exp.(UsingArithmeticInComparison).workingWithPointer() and
not exp.(UsingArithmeticInComparison).insideTheLoop() and
not exp.(UsingArithmeticInComparison).compareWithZero() and
exp.(UsingArithmeticInComparison).compareWithOutZero()
or
exists(WhileStmt wst | wst instanceof UsingWhileAfterWhile and exp = wst.getCondition())
select exp, "this expression needs your attention"

View File

@@ -33,7 +33,7 @@ the break statement only exits from one level of the loop.</p>
</li>
<li>
Mats Henricson and Erik Nyquist, <i>Industrial Strength C++</i>, published by Prentice Hall PTR (1997).
Chapter 4: Control Flow, Rule 4.6 (<a href="http://mongers.org/industrial-c++/">PDF</a>).
Chapter 4: Control Flow, Rule 4.6 (<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">PDF</a>).
</li>
<li>
<a href="http://www.cplusplus.com/doc/tutorial/control/">www.cplusplus.com Control Structures</a>

View File

@@ -39,7 +39,7 @@ loop if the loop requires more complicated variable iteration.
</li>
<li>
Mats Henricson and Erik Nyquist, <i>Industrial Strength C++</i>, published by Prentice Hall PTR (1997).
Chapter 4: Control Flow, Rule 4.1 (<a href="http://mongers.org/industrial-c++/">PDF</a>).
Chapter 4: Control Flow, Rule 4.1 (<a href="https://web.archive.org/web/20190919025638/https://mongers.org/industrial-c++/">PDF</a>).
</li>

View File

@@ -101,6 +101,7 @@ class Type extends Locatable, @type {
*
* For example, starting with `const i64* const` in the context of `typedef long long i64;`, this predicate will return `long long*`.
*/
pragma[nomagic]
Type getUnspecifiedType() { unspecifiedtype(underlyingElement(this), unresolveElement(result)) }
/**

View File

@@ -14,6 +14,7 @@ import cpp
* In rare cases, the same node is used in multiple control-flow scopes. This
* confuses the dominance analysis, so this predicate is used to exclude them.
*/
pragma[noinline]
private predicate hasMultiScopeNode(Function f) {
exists(ControlFlowNode node |
node.getControlFlowScope() = f and

View File

@@ -1307,7 +1307,8 @@ private predicate conditionJumps(Expr test, boolean truth, Node n2, Pos p2) {
)
}
// Factored out for performance. See QL-796.
// Pulled out for performance. See
// https://github.com/github/codeql-coreql-team/issues/1044.
private predicate normalGroupMemberBaseCase(Node memberNode, Pos memberPos, Node atNode) {
memberNode = atNode and
memberPos.isAt() and

View File

@@ -104,9 +104,43 @@ private predicate loopConditionAlwaysUponEntry(ControlFlowNode loop, Expr condit
)
}
/**
* This relation is the same as the `el instanceof Function`, only obfuscated
* so the optimizer will not understand that any `FunctionCall.getTarget()`
* should be in this relation.
*/
pragma[noinline]
private predicate isFunction(Element el) {
el instanceof Function
or
el.(Expr).getParent() = el
}
/**
* Holds if `fc` is a `FunctionCall` with no return value for `getTarget`. This
* can happen in case of rare database inconsistencies.
*/
pragma[noopt]
private predicate callHasNoTarget(@funbindexpr fc) {
exists(Function f |
funbind(fc, f) and
not isFunction(f)
)
}
// Pulled out for performance. See
// https://github.com/github/codeql-coreql-team/issues/1044.
private predicate potentiallyReturningFunctionCall_base(FunctionCall fc) {
fc.isVirtual()
or
callHasNoTarget(fc)
}
/** A function call that *may* return; if in doubt, we assume it may. */
private predicate potentiallyReturningFunctionCall(FunctionCall fc) {
potentiallyReturningFunction(fc.getTarget()) or fc.isVirtual()
potentiallyReturningFunctionCall_base(fc)
or
potentiallyReturningFunction(fc.getTarget())
}
/** A function that *may* return; if in doubt, we assume it may. */

View File

@@ -31,7 +31,7 @@ predicate accessPathCostLimits(int apLimit, int tupleLimit) {
* currently excludes read-steps, store-steps, and flow-through.
*
* The analysis uses non-linear recursion: When computing a flow path in or out
* of a call, we use the results of the analysis recursively to resolve lamba
* of a call, we use the results of the analysis recursively to resolve lambda
* calls. For this reason, we cannot reuse the code from `DataFlowImpl.qll` directly.
*/
private module LambdaFlow {

View File

@@ -321,5 +321,5 @@ predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c)
/** Holds if `call` is a lambda call of kind `kind` where `receiver` is the lambda expression. */
predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { none() }
/** Extra data-flow steps needed for lamba flow analysis. */
/** Extra data-flow steps needed for lambda flow analysis. */
predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() }

View File

@@ -850,6 +850,24 @@ class NewOrNewArrayExpr extends Expr, @any_new_expr {
this.getAllocatorCall()
.getArgument(this.getAllocator().(OperatorNewAllocationFunction).getPlacementArgument())
}
/**
* For `operator new`, this gets the call or expression that initializes the allocated object, if any.
*
* As examples, for `new int(4)`, this will be `4`, and for `new std::vector(4)`, this will
* be a call to the constructor `std::vector::vector(size_t)` with `4` as an argument.
*
* For `operator new[]`, this gets the call or expression that initializes the first element of the
* array, if any.
*
* This will either be a call to the default constructor for the array's element type (as
* in `new std::string[10]`), or a literal zero for arrays of scalars which are zero-initialized
* due to extra parentheses (as in `new int[10]()`).
*
* At runtime, the constructor will be called once for each element in the array, but the
* constructor call only exists once in the AST.
*/
final Expr getInitializer() { result = this.getChild(1) }
}
/**
@@ -871,14 +889,6 @@ class NewExpr extends NewOrNewArrayExpr, @new_expr {
override Type getAllocatedType() {
new_allocated_type(underlyingElement(this), unresolveElement(result))
}
/**
* Gets the call or expression that initializes the allocated object, if any.
*
* As examples, for `new int(4)`, this will be `4`, and for `new std::vector(4)`, this will
* be a call to the constructor `std::vector::vector(size_t)` with `4` as an argument.
*/
Expr getInitializer() { result = this.getChild(1) }
}
/**
@@ -909,18 +919,6 @@ class NewArrayExpr extends NewOrNewArrayExpr, @new_array_expr {
result = getType().getUnderlyingType().(PointerType).getBaseType()
}
/**
* Gets the call or expression that initializes the first element of the array, if any.
*
* This will either be a call to the default constructor for the array's element type (as
* in `new std::string[10]`), or a literal zero for arrays of scalars which are zero-initialized
* due to extra parentheses (as in `new int[10]()`).
*
* At runtime, the constructor will be called once for each element in the array, but the
* constructor call only exists once in the AST.
*/
Expr getInitializer() { result = this.getChild(1) }
/**
* Gets the extent of the non-constant array dimension, if any.
*
@@ -1271,7 +1269,8 @@ private predicate convparents(Expr child, int idx, Element parent) {
)
}
// Pulled out for performance. See QL-796.
// Pulled out for performance. See
// https://github.com/github/codeql-coreql-team/issues/1044.
private predicate hasNoConversions(Expr e) { not e.hasConversion() }
/**

View File

@@ -31,7 +31,7 @@ predicate accessPathCostLimits(int apLimit, int tupleLimit) {
* currently excludes read-steps, store-steps, and flow-through.
*
* The analysis uses non-linear recursion: When computing a flow path in or out
* of a call, we use the results of the analysis recursively to resolve lamba
* of a call, we use the results of the analysis recursively to resolve lambda
* calls. For this reason, we cannot reuse the code from `DataFlowImpl.qll` directly.
*/
private module LambdaFlow {

View File

@@ -557,5 +557,5 @@ predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c)
/** Holds if `call` is a lambda call of kind `kind` where `receiver` is the lambda expression. */
predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { none() }
/** Extra data-flow steps needed for lamba flow analysis. */
/** Extra data-flow steps needed for lambda flow analysis. */
predicate additionalLambdaFlowStep(Node nodeFrom, Node nodeTo, boolean preservesValue) { none() }

View File

@@ -297,7 +297,8 @@ class Instruction extends Construction::TStageInstruction {
/**
* Gets the opcode that specifies the operation performed by this instruction.
*/
final Opcode getOpcode() { result = Construction::getInstructionOpcode(this) }
pragma[inline]
final Opcode getOpcode() { Construction::getInstructionOpcode(result, this) }
/**
* Gets all direct uses of the result of this instruction. The result can be

View File

@@ -338,15 +338,21 @@ private module Cached {
instr = unreachedInstruction(_) and result = Language::getVoidType()
}
/**
* Holds if `opcode` is the opcode that specifies the operation performed by `instr`.
*
* The parameters are ordered such that they produce a clean join (with no need for reordering)
* in the characteristic predicates of the `Instruction` subclasses.
*/
cached
Opcode getInstructionOpcode(Instruction instr) {
result = getOldInstruction(instr).getOpcode()
predicate getInstructionOpcode(Opcode opcode, Instruction instr) {
opcode = getOldInstruction(instr).getOpcode()
or
instr = phiInstruction(_, _) and result instanceof Opcode::Phi
instr = phiInstruction(_, _) and opcode instanceof Opcode::Phi
or
instr = chiInstruction(_) and result instanceof Opcode::Chi
instr = chiInstruction(_) and opcode instanceof Opcode::Chi
or
instr = unreachedInstruction(_) and result instanceof Opcode::Unreached
instr = unreachedInstruction(_) and opcode instanceof Opcode::Unreached
}
cached

View File

@@ -297,7 +297,8 @@ class Instruction extends Construction::TStageInstruction {
/**
* Gets the opcode that specifies the operation performed by this instruction.
*/
final Opcode getOpcode() { result = Construction::getInstructionOpcode(this) }
pragma[inline]
final Opcode getOpcode() { Construction::getInstructionOpcode(result, this) }
/**
* Gets all direct uses of the result of this instruction. The result can be

View File

@@ -360,8 +360,8 @@ CppType getInstructionResultType(TStageInstruction instr) {
getInstructionTranslatedElement(instr).hasInstruction(_, getInstructionTag(instr), result)
}
Opcode getInstructionOpcode(TStageInstruction instr) {
getInstructionTranslatedElement(instr).hasInstruction(result, getInstructionTag(instr), _)
predicate getInstructionOpcode(Opcode opcode, TStageInstruction instr) {
getInstructionTranslatedElement(instr).hasInstruction(opcode, getInstructionTag(instr), _)
}
IRFunctionBase getInstructionEnclosingIRFunction(TStageInstruction instr) {

View File

@@ -42,7 +42,8 @@ IRTempVariable getIRTempVariable(Locatable ast, TempVariableTag tag) {
*/
predicate isIRConstant(Expr expr) { exists(expr.getValue()) }
// Pulled out to work around QL-796
// Pulled out for performance. See
// https://github.com/github/codeql-coreql-team/issues/1044.
private predicate isOrphan(Expr expr) { not exists(getRealParent(expr)) }
/**

View File

@@ -297,7 +297,8 @@ class Instruction extends Construction::TStageInstruction {
/**
* Gets the opcode that specifies the operation performed by this instruction.
*/
final Opcode getOpcode() { result = Construction::getInstructionOpcode(this) }
pragma[inline]
final Opcode getOpcode() { Construction::getInstructionOpcode(result, this) }
/**
* Gets all direct uses of the result of this instruction. The result can be

View File

@@ -338,15 +338,21 @@ private module Cached {
instr = unreachedInstruction(_) and result = Language::getVoidType()
}
/**
* Holds if `opcode` is the opcode that specifies the operation performed by `instr`.
*
* The parameters are ordered such that they produce a clean join (with no need for reordering)
* in the characteristic predicates of the `Instruction` subclasses.
*/
cached
Opcode getInstructionOpcode(Instruction instr) {
result = getOldInstruction(instr).getOpcode()
predicate getInstructionOpcode(Opcode opcode, Instruction instr) {
opcode = getOldInstruction(instr).getOpcode()
or
instr = phiInstruction(_, _) and result instanceof Opcode::Phi
instr = phiInstruction(_, _) and opcode instanceof Opcode::Phi
or
instr = chiInstruction(_) and result instanceof Opcode::Chi
instr = chiInstruction(_) and opcode instanceof Opcode::Chi
or
instr = unreachedInstruction(_) and result instanceof Opcode::Unreached
instr = unreachedInstruction(_) and opcode instanceof Opcode::Unreached
}
cached

View File

@@ -1617,6 +1617,20 @@ private module SimpleRangeAnalysisCached {
defMightOverflowPositively(def, v)
}
/**
* Holds if `e` is an expression where the concept of overflow makes sense.
* This predicate is used to filter out some of the unanalyzable expressions
* from `exprMightOverflowPositively` and `exprMightOverflowNegatively`.
*/
pragma[inline]
private predicate exprThatCanOverflow(Expr e) {
e instanceof UnaryArithmeticOperation or
e instanceof BinaryArithmeticOperation or
e instanceof AssignArithmeticOperation or
e instanceof LShiftExpr or
e instanceof AssignLShiftExpr
}
/**
* Holds if the expression might overflow negatively. This predicate
* does not consider the possibility that the expression might overflow
@@ -1630,6 +1644,11 @@ private module SimpleRangeAnalysisCached {
// bound of `x`, so the standard logic (above) does not work for
// detecting whether it might overflow.
getLowerBoundsImpl(expr.(PostfixDecrExpr)) = exprMinVal(expr)
or
// We can't conclude that any unanalyzable expression might overflow. This
// is because there are many expressions that the range analysis doesn't
// handle, but where the concept of overflow doesn't make sense.
exprThatCanOverflow(expr) and not analyzableExpr(expr)
}
/**
@@ -1657,6 +1676,11 @@ private module SimpleRangeAnalysisCached {
// bound of `x`, so the standard logic (above) does not work for
// detecting whether it might overflow.
getUpperBoundsImpl(expr.(PostfixIncrExpr)) = exprMaxVal(expr)
or
// We can't conclude that any unanalyzable expression might overflow. This
// is because there are many expressions that the range analysis doesn't
// handle, but where the concept of overflow doesn't make sense.
exprThatCanOverflow(expr) and not analyzableExpr(expr)
}
/**

View File

@@ -99,12 +99,11 @@ VariableAccess varUse(LocalScopeVariable v) { result = v.getAnAccess() }
* Holds if `e` potentially overflows and `use` is an operand of `e` that is not guarded.
*/
predicate missingGuardAgainstOverflow(Operation e, VariableAccess use) {
(
convertedExprMightOverflowPositively(e)
or
// Ensure that the predicate holds when range analysis cannot determine an upper bound
upperBound(e.getFullyConverted()) = exprMaxVal(e.getFullyConverted())
) and
// Since `e` is guarenteed to be a `BinaryArithmeticOperation`, a `UnaryArithmeticOperation` or
// an `AssignArithmeticOperation` by the other constraints in this predicate, we know that
// `convertedExprMightOverflowPositively` will have a result even when `e` is not analyzable
// by `SimpleRangeAnalysis`.
convertedExprMightOverflowPositively(e) and
use = e.getAnOperand() and
exists(LocalScopeVariable v | use.getTarget() = v |
// overflow possible if large
@@ -126,12 +125,11 @@ predicate missingGuardAgainstOverflow(Operation e, VariableAccess use) {
* Holds if `e` potentially underflows and `use` is an operand of `e` that is not guarded.
*/
predicate missingGuardAgainstUnderflow(Operation e, VariableAccess use) {
(
convertedExprMightOverflowNegatively(e)
or
// Ensure that the predicate holds when range analysis cannot determine a lower bound
lowerBound(e.getFullyConverted()) = exprMinVal(e.getFullyConverted())
) and
// Since `e` is guarenteed to be a `BinaryArithmeticOperation`, a `UnaryArithmeticOperation` or
// an `AssignArithmeticOperation` by the other constraints in this predicate, we know that
// `convertedExprMightOverflowNegatively` will have a result even when `e` is not analyzable
// by `SimpleRangeAnalysis`.
convertedExprMightOverflowNegatively(e) and
use = e.getAnOperand() and
exists(LocalScopeVariable v | use.getTarget() = v |
// underflow possible if use is left operand and small

View File

@@ -0,0 +1,18 @@
| test.cpp:21:9:21:15 | new | This allocation cannot return null. $@ is unnecessary. | test.cpp:21:9:21:15 | new | This check |
| test.cpp:29:13:29:24 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:30:7:30:13 | ... == ... | This check |
| test.cpp:33:13:33:24 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:34:8:34:9 | p2 | This check |
| test.cpp:37:13:37:24 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:38:7:38:16 | ... == ... | This check |
| test.cpp:41:13:41:24 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:42:7:42:19 | ... == ... | This check |
| test.cpp:45:13:45:24 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:46:7:46:8 | p5 | This check |
| test.cpp:49:8:49:19 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:50:7:50:13 | ... == ... | This check |
| test.cpp:53:8:53:19 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:54:8:54:9 | p7 | This check |
| test.cpp:58:8:58:19 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:59:7:59:16 | ... == ... | This check |
| test.cpp:63:8:63:19 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:64:7:64:19 | ... != ... | This check |
| test.cpp:69:9:69:20 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:70:7:70:14 | ... != ... | This check |
| test.cpp:75:11:75:22 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:76:13:76:15 | p11 | This check |
| test.cpp:92:5:92:31 | new[] | This allocation cannot throw. $@ is unnecessary. | test.cpp:97:36:98:3 | { ... } | This catch block |
| test.cpp:93:15:93:41 | new[] | This allocation cannot throw. $@ is unnecessary. | test.cpp:97:36:98:3 | { ... } | This catch block |
| test.cpp:96:10:96:36 | new[] | This allocation cannot throw. $@ is unnecessary. | test.cpp:97:36:98:3 | { ... } | This catch block |
| test.cpp:151:9:151:24 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:152:15:152:18 | { ... } | This catch block |
| test.cpp:199:15:199:35 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:201:16:201:19 | { ... } | This catch block |
| test.cpp:212:14:212:34 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:213:34:213:36 | { ... } | This catch block |

View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-570/IncorrectAllocationErrorHandling.ql

View File

@@ -1,5 +0,0 @@
| test.cpp:30:15:30:26 | call to operator new[] | memory allocation error check is incorrect or missing |
| test.cpp:38:9:38:20 | call to operator new[] | memory allocation error check is incorrect or missing |
| test.cpp:50:13:50:38 | call to operator new[] | memory allocation error check is incorrect or missing |
| test.cpp:51:22:51:47 | call to operator new[] | memory allocation error check is incorrect or missing |
| test.cpp:53:18:53:43 | call to operator new[] | memory allocation error check is incorrect or missing |

View File

@@ -1 +0,0 @@
experimental/Security/CWE/CWE-570/WrongInDetectingAndHandlingMemoryAllocationErrors.ql

View File

@@ -1,97 +1,227 @@
#define NULL ((void*)0)
class exception {};
#define NULL ((void *)0)
namespace std{
struct nothrow_t {};
typedef unsigned long size_t;
class bad_alloc{
const char* what() const throw();
};
extern const std::nothrow_t nothrow;
}
namespace std {
struct nothrow_t {};
typedef unsigned long size_t;
class exception {};
class bad_alloc : public exception {};
extern const std::nothrow_t nothrow;
} // namespace std
using namespace std;
void* operator new(std::size_t _Size);
void* operator new[](std::size_t _Size);
void* operator new( std::size_t count, const std::nothrow_t& tag ) noexcept;
void* operator new[]( std::size_t count, const std::nothrow_t& tag ) noexcept;
void *operator new(std::size_t);
void *operator new[](std::size_t);
void *operator new(std::size_t, const std::nothrow_t &) noexcept;
void *operator new[](std::size_t, const std::nothrow_t &) noexcept;
void badNew_0_0()
{
while (true) {
new int[100]; // BAD [NOT DETECTED]
if(!(new int[100])) // BAD [NOT DETECTED]
return;
}
}
void badNew_0_1()
{
int * i = new int[100]; // BAD
if(i == 0)
return;
if(!i)
return;
if(i == NULL)
return;
int * j;
j = new int[100]; // BAD
if(j == 0)
return;
if(!j)
return;
if(j == NULL)
return;
}
void badNew_1_0()
{
try {
while (true) {
new(std::nothrow) int[100]; // BAD
int* p = new(std::nothrow) int[100]; // BAD
int* p1;
p1 = new(std::nothrow) int[100]; // BAD
}
} catch (const exception &){//const std::bad_alloc& e) {
// std::cout << e.what() << '\n';
}
}
void badNew_1_1()
{
while (true) {
int* p = new(std::nothrow) int[100]; // BAD [NOT DETECTED]
new(std::nothrow) int[100]; // BAD [NOT DETECTED]
}
void bad_new_in_condition() {
if (!(new int)) { // BAD
return;
}
}
void goodNew_0_0()
{
try {
while (true) {
new int[100]; // GOOD
}
} catch (const exception &){//const std::bad_alloc& e) {
// std::cout << e.what() << '\n';
}
void foo(int**);
void bad_new_missing_exception_handling() {
int *p1 = new int[100]; // BAD
if (p1 == 0)
return;
int *p2 = new int[100]; // BAD
if (!p2)
return;
int *p3 = new int[100]; // BAD
if (p3 == NULL)
return;
int *p4 = new int[100]; // BAD
if (p4 == nullptr)
return;
int *p5 = new int[100]; // BAD
if (p5) {} else return;
int *p6;
p6 = new int[100]; // BAD
if (p6 == 0) return;
int *p7;
p7 = new int[100]; // BAD
if (!p7)
return;
int *p8;
p8 = new int[100]; // BAD
if (p8 == NULL)
return;
int *p9;
p9 = new int[100]; // BAD
if (p9 != nullptr) {
} else
return;
int *p10;
p10 = new int[100]; // BAD
if (p10 != 0) {
}
int *p11;
do {
p11 = new int[100]; // BAD
} while (!p11);
int* p12 = new int[100];
foo(&p12);
if(p12) {} else return; // GOOD: p12 is probably modified in foo, so it's
// not the return value of the new that's checked.
int* p13 = new int[100];
foo(&p13);
if(!p13) {
return;
} else { }; // GOOD: same as above.
}
void goodNew_1_0()
{
while (true) {
int* p = new(std::nothrow) int[100]; // GOOD
if (p == nullptr) {
// std::cout << "Allocation returned nullptr\n";
break;
}
int* p1;
p1 = new(std::nothrow) int[100]; // GOOD
if (p1 == nullptr) {
// std::cout << "Allocation returned nullptr\n";
break;
}
if (new(std::nothrow) int[100] == nullptr) { // GOOD
// std::cout << "Allocation returned nullptr\n";
break;
}
}
void bad_new_nothrow_in_exception_body() {
try {
new (std::nothrow) int[100]; // BAD
int *p1 = new (std::nothrow) int[100]; // BAD
int *p2;
p2 = new (std::nothrow) int[100]; // BAD
} catch (const std::bad_alloc &) {
}
}
void good_new_has_exception_handling() {
try {
int *p1 = new int[100]; // GOOD
} catch (...) {
}
}
void good_new_handles_nullptr() {
int *p1 = new (std::nothrow) int[100]; // GOOD
if (p1 == nullptr)
return;
int *p2;
p2 = new (std::nothrow) int[100]; // GOOD
if (p2 == nullptr)
return;
int *p3;
p3 = new (std::nothrow) int[100]; // GOOD
if (p3 != nullptr) {
}
int *p4;
p4 = new (std::nothrow) int[100]; // GOOD
if (p4) {
} else
return;
int *p5;
p5 = new (std::nothrow) int[100]; // GOOD
if (p5 != nullptr) {
} else
return;
if (new (std::nothrow) int[100] == nullptr)
return; // GOOD
}
void* operator new(std::size_t count, void*) noexcept;
void* operator new[](std::size_t count, void*) noexcept;
struct Foo {
Foo() noexcept;
Foo(int);
operator bool();
};
void bad_placement_new_with_exception_handling() {
char buffer[1024];
try { new (buffer) Foo; } // BAD
catch (...) { }
}
void good_placement_new_with_exception_handling() {
char buffer[1024];
try { new (buffer) Foo(42); } // GOOD: Foo constructor might throw
catch (...) { }
}
int unknown_value_without_exceptions() noexcept;
void may_throw() {
if(unknown_value_without_exceptions()) {
throw "bad luck exception!";
}
}
void unknown_code_that_may_throw(int*);
void unknown_code_that_will_not_throw(int*) noexcept;
void calls_throwing_code(int* p) {
if(unknown_value_without_exceptions()) unknown_code_that_may_throw(p);
}
void calls_non_throwing(int* p) {
if (unknown_value_without_exceptions()) unknown_code_that_will_not_throw(p);
}
void good_new_with_throwing_call() {
try {
int* p1 = new(std::nothrow) int; // GOOD
may_throw();
} catch(...) { }
try {
int* p2 = new(std::nothrow) int; // GOOD
Foo f(10);
} catch(...) { }
try {
int* p3 = new(std::nothrow) int; // GOOD
calls_throwing_code(p3);
} catch(...) { }
}
void bad_new_with_nonthrowing_call() {
try {
int* p1 = new(std::nothrow) int; // BAD
calls_non_throwing(p1);
} catch(...) { }
try {
int* p2 = new(std::nothrow) int; // GOOD: boolean conversion constructor might throw
Foo f;
if(f) { }
} catch(...) { }
}
void bad_new_catch_baseclass_of_bad_alloc() {
try {
int* p = new(std::nothrow) int; // BAD
} catch(const std::exception&) { }
}
void good_new_catch_exception_in_assignment() {
int* p;
try {
p = new int; // GOOD
} catch(const std::bad_alloc&) { }
}
void good_new_catch_exception_in_conversion() {
try {
long* p = (long*) new int; // GOOD
} catch(const std::bad_alloc&) { }
}

View File

@@ -0,0 +1,4 @@
| test.c:15:6:15:16 | ... + ... | this expression needs your attention |
| test.c:17:17:17:27 | ... + ... | this expression needs your attention |
| test.c:22:10:22:15 | ... > ... | this expression needs your attention |
| test.c:26:10:26:15 | ... > ... | this expression needs your attention |

View File

@@ -0,0 +1 @@
experimental/Security/CWE/CWE-691/InsufficientControlFlowManagementAfterRefactoringTheCode.ql

View File

@@ -12,14 +12,18 @@ void workFunction_0(char *s) {
void workFunction_1(char *s) {
int intA,intB;
if(intA + intB) return; // BAD [NOT DETECTED]
if(intA + intB) return; // BAD
if(intA + intB>4) return; // GOOD
if(intA>0 && (intA + intB)) return; // BAD [NOT DETECTED]
if(intA>0 && (intA + intB)) return; // BAD
while(intA>0)
{
if(intB - intA<10) break;
intA--;
}while(intA>0); // BAD [NOT DETECTED]
}while(intA>0); // BAD
for(intA=100; intA>0; intA--)
{
if(intB - intA<10) break;
}while(intA>0); // BAD
while(intA>0)
{
if(intB - intA<10) break;

View File

@@ -3,5 +3,4 @@
| test.c:50:3:50:5 | sc3 | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:49:9:49:16 | 127 | Extreme value |
| test.c:59:3:59:5 | sc6 | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:58:9:58:16 | 127 | Extreme value |
| test.c:63:3:63:5 | sc8 | $@ flows to here and is used in arithmetic, potentially causing an underflow. | test.c:62:9:62:16 | - ... | Extreme value |
| test.c:75:3:75:5 | sc1 | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:74:9:74:16 | 127 | Extreme value |
| test.c:124:9:124:9 | x | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:118:17:118:23 | 2147483647 | Extreme value |

View File

@@ -72,7 +72,7 @@ void test_negatives() {
signed char sc1, sc2, sc3, sc4, sc5, sc6, sc7, sc8;
sc1 = CHAR_MAX;
sc1 += 0; // GOOD [FALSE POSITIVE]
sc1 += 0; // GOOD
sc1 += -1; // GOOD
sc2 = CHAR_MIN;
sc2 += -1; // BAD [NOT DETECTED]

View File

@@ -1,10 +1,15 @@
| test.cpp:6:5:6:13 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:10:8:10:24 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:15:9:15:25 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:32:12:32:20 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:39:12:39:20 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:47:5:47:13 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:55:5:55:13 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:62:5:62:13 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:69:5:69:13 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:75:8:75:16 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:101:6:101:14 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:128:6:128:14 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:137:6:137:14 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:146:7:146:15 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:152:7:152:15 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:182:6:182:14 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:208:6:208:14 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:252:10:252:18 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:266:10:266:24 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:276:11:276:19 | ... > ... | Unsigned subtraction can never be negative. |
| test.cpp:288:10:288:18 | ... > ... | Unsigned subtraction can never be negative. |

View File

@@ -12,7 +12,7 @@ void test(unsigned x, unsigned y, bool unknown) {
}
if(total <= limit) {
while(limit - total > 0) { // GOOD [FALSE POSITIVE]
while(limit - total > 0) { // GOOD
total += getAnInt();
if(total > limit) break;
}
@@ -29,14 +29,14 @@ void test(unsigned x, unsigned y, bool unknown) {
} else {
y = x;
}
bool b1 = x - y > 0; // GOOD [FALSE POSITIVE]
bool b1 = x - y > 0; // GOOD
x = getAnInt();
y = getAnInt();
if(y > x) {
y = x - 1;
}
bool b2 = x - y > 0; // GOOD [FALSE POSITIVE]
bool b2 = x - y > 0; // GOOD
int N = getAnInt();
y = x;
@@ -44,7 +44,7 @@ void test(unsigned x, unsigned y, bool unknown) {
if(unknown) { y--; }
}
if(x - y > 0) { } // GOOD [FALSE POSITIVE]
if(x - y > 0) { } // GOOD
x = y;
while(cond()) {
@@ -52,7 +52,7 @@ void test(unsigned x, unsigned y, bool unknown) {
y--;
}
if(x - y > 0) { } // GOOD [FALSE POSITIVE]
if(x - y > 0) { } // GOOD
y = 0;
for(int i = 0; i < x; ++i) {
@@ -66,7 +66,7 @@ void test(unsigned x, unsigned y, bool unknown) {
if(unknown) { x++; }
}
if(x - y > 0) { } // GOOD [FALSE POSITIVE]
if(x - y > 0) { } // GOOD
int n = getAnInt();
if (n > x - y) { n = x - y; }
@@ -74,4 +74,227 @@ void test(unsigned x, unsigned y, bool unknown) {
y += n; // NOTE: `n` is at most `x - y` at this point.
if (x - y > 0) {} // GOOD [FALSE POSITIVE]
}
}
}
void test2() {
unsigned int a = getAnInt();
unsigned int b = a;
if (a - b > 0) { // GOOD (as a = b)
// ...
}
}
void test3() {
unsigned int a = getAnInt();
unsigned int b = a - 1;
if (a - b > 0) { // GOOD (as a >= b)
// ...
}
}
void test4() {
unsigned int a = getAnInt();
unsigned int b = a + 1;
if (a - b > 0) { // BAD
// ...
}
}
void test5() {
unsigned int b = getAnInt();
unsigned int a = b;
if (a - b > 0) { // GOOD (as a = b)
// ...
}
}
void test6() {
unsigned int b = getAnInt();
unsigned int a = b + 1;
if (a - b > 0) { // GOOD (as a >= b)
// ...
}
}
void test7() {
unsigned int b = getAnInt();
unsigned int a = b - 1;
if (a - b > 0) { // BAD
// ...
}
}
void test8() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (a - b > 0) { // BAD
// ...
}
if (a >= b) { // GOOD
if (a - b > 0) { // GOOD (as a >= b)
// ...
}
} else {
if (a - b > 0) { // BAD
// ...
}
}
if (b >= a) { // GOOD
if (a - b > 0) { // BAD
// ...
}
} else {
if (a - b > 0) { // GOOD (as a > b)
// ...
}
}
while (a >= b) { // GOOD
if (a - b > 0) { // GOOD (as a >= b)
// ...
}
}
if (a < b) return;
if (a - b > 0) { // GOOD (as a >= b)
// ...
}
}
void test9() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (a < b) {
b = 0;
}
if (a - b > 0) { // GOOD (as a >= b) [FALSE POSITIVE]
// ...
}
}
void test10() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (a < b) {
a = b;
}
if (a - b > 0) { // GOOD (as a >= b)
// ...
}
}
void test11() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (a < b) return;
b = getAnInt();
if (a - b > 0) { // BAD
// ...
}
}
void test12() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
unsigned int c;
if ((b <= c) && (c <= a)) {
if (a - b > 0) { // GOOD (as b <= a)
// ...
}
}
if (b <= c) {
if (c <= a) {
if (a - b > 0) { // GOOD (as b <= a)
// ...
}
}
}
}
int test13() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (b != 0) {
return 0;
} // b = 0
return (a - b > 0); // GOOD (as b = 0)
}
int test14() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (!b) {
return 0;
} // b != 0
return (a - b > 0); // BAD
}
struct Numbers
{
unsigned int a, b;
};
int test15(Numbers *n) {
if (!n) {
return 0;
}
return (n->a - n->b > 0); // BAD
}
int test16() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (!b) {
return 0;
} else {
return (a - b > 0); // BAD
}
}
int test17() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (b == 0) {
return 0;
} // b != 0
return (a - b > 0); // BAD
}
int test18() {
unsigned int a = getAnInt();
unsigned int b = getAnInt();
if (b) {
return 0;
} // b == 0
return (a - b > 0); // GOOD (as b = 0)
}