Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

C++: Add basic modeling of functions that don't throw #17298

Merged
merged 4 commits into from
Aug 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cpp/ql/lib/semmle/code/cpp/models/Models.qll
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ private import implementations.Accept
private import implementations.Poll
private import implementations.Select
private import implementations.MySql
private import implementations.NoexceptFunction
private import implementations.ODBC
private import implementations.SqLite3
private import implementations.PostgreSql
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,20 @@
* functions. See `semmle.code.cpp.models.Models` for usage information.
*/

import semmle.code.cpp.Function

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.ArrayFunction
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.DataFlow
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.Alias
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.SideEffect
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.Taint
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.NonThrowing
.
import semmle.code.cpp.models.interfaces.ArrayFunction
import semmle.code.cpp.models.interfaces.DataFlow
import semmle.code.cpp.models.interfaces.Alias
import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.Taint
import semmle.code.cpp.models.interfaces.NonThrowing

/**
* The standard functions `memcpy`, `memmove` and `bcopy`; and the gcc variant
* `__builtin___memcpy_chk`.
*/
private class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffectFunction,
AliasFunction
AliasFunction, NonThrowingFunction
{
MemcpyFunction() {
// memcpy(dest, src, num)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@
* functions. See `semmle.code.cpp.models.Models` for usage information.
*/

import semmle.code.cpp.Function

Check warning

Code scanning / CodeQL

Redundant import Warning

Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.ArrayFunction
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.DataFlow
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.Alias
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.SideEffect
.
Redundant import, the module is already imported inside
semmle.code.cpp.models.interfaces.NonThrowing
.
import semmle.code.cpp.models.interfaces.ArrayFunction
import semmle.code.cpp.models.interfaces.DataFlow
import semmle.code.cpp.models.interfaces.Alias
import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.NonThrowing

private class MemsetFunctionModel extends ArrayFunction, DataFlowFunction, AliasFunction,
SideEffectFunction
SideEffectFunction, NonThrowingFunction
{
MemsetFunctionModel() {
this.hasGlobalOrStdOrBslName("memset")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import semmle.code.cpp.models.interfaces.NonThrowing

/**
* A function that is annotated with a `noexcept` specifier (or the equivalent
* `throw()` specifier) guaranteeing that the function can not throw exceptions.
*
* Note: The `throw` specifier was deprecated in C++11 and removed in C++17.
*/
class NoexceptFunction extends NonThrowingFunction {
NoexceptFunction() { this.isNoExcept() or this.isNoThrow() }
}
11 changes: 6 additions & 5 deletions cpp/ql/lib/semmle/code/cpp/models/implementations/Printf.qll
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@
import semmle.code.cpp.models.interfaces.FormattingFunction
import semmle.code.cpp.models.interfaces.Alias
import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.NonThrowing

/**
* The standard functions `printf`, `wprintf` and their glib variants.
*/
private class Printf extends FormattingFunction, AliasFunction {
private class Printf extends FormattingFunction, AliasFunction, NonThrowingFunction {
Printf() {
this instanceof TopLevelFunction and
(
Expand All @@ -36,7 +37,7 @@ private class Printf extends FormattingFunction, AliasFunction {
/**
* The standard functions `fprintf`, `fwprintf` and their glib variants.
*/
private class Fprintf extends FormattingFunction {
private class Fprintf extends FormattingFunction, NonThrowingFunction {
Fprintf() {
this instanceof TopLevelFunction and
(
Expand All @@ -54,7 +55,7 @@ private class Fprintf extends FormattingFunction {
/**
* The standard function `sprintf` and its Microsoft and glib variants.
*/
private class Sprintf extends FormattingFunction {
private class Sprintf extends FormattingFunction, NonThrowingFunction {
Sprintf() {
this instanceof TopLevelFunction and
(
Expand Down Expand Up @@ -97,7 +98,7 @@ private class Sprintf extends FormattingFunction {
/**
* Implements `Snprintf`.
*/
private class SnprintfImpl extends Snprintf, AliasFunction, SideEffectFunction {
private class SnprintfImpl extends Snprintf, AliasFunction, SideEffectFunction, NonThrowingFunction {
SnprintfImpl() {
this instanceof TopLevelFunction and
(
Expand Down Expand Up @@ -204,7 +205,7 @@ private class StringCchPrintf extends FormattingFunction {
/**
* The standard function `syslog`.
*/
private class Syslog extends FormattingFunction {
private class Syslog extends FormattingFunction, NonThrowingFunction {
Syslog() {
this instanceof TopLevelFunction and
this.hasGlobalName("syslog") and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ import semmle.code.cpp.models.interfaces.ArrayFunction
import semmle.code.cpp.models.interfaces.DataFlow
import semmle.code.cpp.models.interfaces.Taint
import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.NonThrowing

/**
* The standard function `strcat` and its wide, sized, and Microsoft variants.
*
* Does not include `strlcat`, which is covered by `StrlcatFunction`
*/
class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, SideEffectFunction {
class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, SideEffectFunction,
NonThrowingFunction
{
StrcatFunction() {
this.hasGlobalOrStdOrBslName([
"strcat", // strcat(dst, src)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ import semmle.code.cpp.models.interfaces.ArrayFunction
import semmle.code.cpp.models.interfaces.DataFlow
import semmle.code.cpp.models.interfaces.Taint
import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.NonThrowing

/**
* The standard function `strcpy` and its wide, sized, and Microsoft variants.
*/
class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, SideEffectFunction {
class StrcpyFunction extends ArrayFunction, DataFlowFunction, TaintFunction, SideEffectFunction,
NonThrowingFunction
{
StrcpyFunction() {
this.hasGlobalOrStdOrBslName([
"strcpy", // strcpy(dst, src)
Expand Down
11 changes: 11 additions & 0 deletions cpp/ql/lib/semmle/code/cpp/models/interfaces/NonThrowing.qll
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/**
* Provides an abstract class for modeling functions that never throw.
*/

import semmle.code.cpp.Function
import semmle.code.cpp.models.Models

/**
* A function that is guaranteed to never throw.
*/
abstract class NonThrowingFunction extends Function { }
Fixed Show fixed Hide fixed
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import cpp
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
import semmle.code.cpp.controlflow.Guards
import semmle.code.cpp.models.implementations.NoexceptFunction
geoffw0 marked this conversation as resolved.
Show resolved Hide resolved

/** Gets the `Constructor` invoked when `newExpr` allocates memory. */
Constructor getConstructorForAllocation(NewOrNewArrayExpr newExpr) {
Expand Down Expand Up @@ -44,9 +45,8 @@ predicate deleteMayThrow(DeleteOrDeleteArrayExpr deleteExpr) {
* 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()
not f instanceof NonThrowingFunction and
(not exists(f.getBlock()) or stmtMayThrow(f.getBlock()))
}

/** Holds if the evaluation of `stmt` may throw an exception. */
Expand Down Expand Up @@ -172,8 +172,7 @@ class ThrowingAllocator extends Function {
not exists(Parameter p | p = this.getAParameter() |
p.getUnspecifiedType().stripType() instanceof NoThrowType
) and
not this.isNoExcept() and
not this.isNoThrow()
not this instanceof NoexceptFunction
)
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Add modeling of C functions that don't throw, thereby increasing the precision of the `cpp/incorrect-allocation-error-handling` ("Incorrect allocation-error handling") query. The query now produces additional true positives.
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,4 @@
| test.cpp:229:15:229:35 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:231:16:231:19 | { ... } | This catch block |
| test.cpp:242:14:242:34 | new | This allocation cannot throw. $@ is unnecessary. | test.cpp:243:34:243:36 | { ... } | This catch block |
| test.cpp:276:17:276:31 | new[] | This allocation cannot return null. $@ is unnecessary. | test.cpp:277:8:277:12 | ! ... | This check |
| test.cpp:288:19:288:47 | new[] | This allocation cannot throw. $@ is unnecessary. | test.cpp:291:30:293:5 | { ... } | This catch block |
2 changes: 1 addition & 1 deletion cpp/ql/test/query-tests/Security/CWE/CWE-570/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ namespace qhelp {
}

// BAD: the allocation won't throw an exception, but
// instead return a null pointer. [NOT DETECTED]
// instead return a null pointer.
void bad2(std::size_t length) noexcept {
try {
int* dest = new(std::nothrow) int[length];
Expand Down
Loading