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

Brodes/seh flow phase1 throwing models #18014

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
de05aee
Adding model transition to using Throwing.qll.
bdrodes Nov 18, 2024
4b83a45
Change log
bdrodes Nov 18, 2024
1c7b5ae
Merge branch 'main' into brodes/seh_flow_phase1_throwing_models
bdrodes Nov 18, 2024
792231c
Removing SEH default case for function calls as the logic to handle S…
bdrodes Nov 18, 2024
1c874d3
Fixed usage raisesException
bdrodes Nov 19, 2024
5bb765d
Merge branch 'main' into brodes/seh_flow_phase1_throwing_models
bdrodes Nov 19, 2024
26d590a
Putting back deleted file, and deprecating instead. Deprecating mayTh…
bdrodes Nov 19, 2024
63ddd81
Merge branch 'brodes/seh_flow_phase1_throwing_models' of https://gith…
bdrodes Nov 19, 2024
0784776
bringing back mayThrowException to make it cleaner/easier for backwar…
bdrodes Nov 19, 2024
ae1ed38
Merge branch 'main' into brodes/seh_flow_phase1_throwing_models
bdrodes Nov 19, 2024
a69daa0
Missing change to 'mayThrowException' in StructuredExceptionHandling.qll
bdrodes Nov 19, 2024
23485f1
Merge branch 'brodes/seh_flow_phase1_throwing_models' of https://gith…
bdrodes Nov 19, 2024
4e77756
Changing terminology back to "throws" vs "rasis" for alwaysThrowsExce…
bdrodes Nov 19, 2024
69df07e
Update cpp/ql/lib/change-notes/2024-11-18-throwing-functions.md
bdrodes Nov 20, 2024
6aa7412
Merge branch 'main' into brodes/seh_flow_phase1_throwing_models
bdrodes Nov 20, 2024
9b2590e
Updating PR per review comments. Moving more towards a simplified model.
bdrodes Nov 21, 2024
4412691
Delaying deprecation of ThrowingFunction.
bdrodes Nov 21, 2024
7059fc3
Adding intermediate solution towards deprecating ThrowingFunction
bdrodes Nov 21, 2024
248f1c4
Updating change log
bdrodes Nov 21, 2024
583651b
Missing NonCppThrowingFunction changes in Printf.qll
bdrodes Nov 21, 2024
66cf736
printf formatting.
bdrodes Nov 21, 2024
37365c7
Updating to NonCppThrowingFunction use in IncorrectALlocationErrorHan…
bdrodes Nov 21, 2024
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
4 changes: 4 additions & 0 deletions cpp/ql/lib/change-notes/2024-11-18-throwing-functions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: deprecated
---
* The `NonThrowing` class (`semmle.code.cpp.models.interfaces.NonThrowing`) has been deprecated. Please use the `NonCppThrowingFunction` class instead.
Original file line number Diff line number Diff line change
Expand Up @@ -364,10 +364,14 @@ class TranslatedFunctionCall extends TranslatedCallExpr, TranslatedDirectCall {

final override predicate mayThrowException() {
expr.getTarget().(ThrowingFunction).mayThrowException(_)
or
expr.getTarget() instanceof AlwaysSehThrowingFunction
}
Comment on lines 365 to 369
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be able to remove this predicate completely, but let's leave that as a follow up.


final override predicate mustThrowException() {
expr.getTarget().(ThrowingFunction).mayThrowException(true)
or
expr.getTarget() instanceof AlwaysSehThrowingFunction
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import semmle.code.cpp.models.interfaces.NonThrowing
* `__builtin___memcpy_chk`.
*/
private class MemcpyFunction extends ArrayFunction, DataFlowFunction, SideEffectFunction,
AliasFunction, NonThrowingFunction
AliasFunction, NonCppThrowingFunction
{
MemcpyFunction() {
// memcpy(dest, src, num)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import semmle.code.cpp.models.interfaces.SideEffect
import semmle.code.cpp.models.interfaces.NonThrowing

private class MemsetFunctionModel extends ArrayFunction, DataFlowFunction, AliasFunction,
SideEffectFunction, NonThrowingFunction
SideEffectFunction, NonCppThrowingFunction
{
MemsetFunctionModel() {
this.hasGlobalOrStdOrBslName("memset")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ import semmle.code.cpp.models.interfaces.NonThrowing
*
* Note: The `throw` specifier was deprecated in C++11 and removed in C++17.
*/
class NoexceptFunction extends NonThrowingFunction {
class NoexceptFunction extends NonCppThrowingFunction {
NoexceptFunction() { this.isNoExcept() or this.isNoThrow() }
}
12 changes: 7 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 @@ -13,7 +13,7 @@ import semmle.code.cpp.models.interfaces.NonThrowing
/**
* The standard functions `printf`, `wprintf` and their glib variants.
*/
private class Printf extends FormattingFunction, AliasFunction, NonThrowingFunction {
private class Printf extends FormattingFunction, AliasFunction, NonCppThrowingFunction {
Printf() {
this instanceof TopLevelFunction and
(
Expand All @@ -37,7 +37,7 @@ private class Printf extends FormattingFunction, AliasFunction, NonThrowingFunct
/**
* The standard functions `fprintf`, `fwprintf` and their glib variants.
*/
private class Fprintf extends FormattingFunction, NonThrowingFunction {
private class Fprintf extends FormattingFunction, NonCppThrowingFunction {
Fprintf() {
this instanceof TopLevelFunction and
(
Expand All @@ -55,7 +55,7 @@ private class Fprintf extends FormattingFunction, NonThrowingFunction {
/**
* The standard function `sprintf` and its Microsoft and glib variants.
*/
private class Sprintf extends FormattingFunction, NonThrowingFunction {
private class Sprintf extends FormattingFunction, NonCppThrowingFunction {
Sprintf() {
this instanceof TopLevelFunction and
(
Expand Down Expand Up @@ -98,7 +98,9 @@ private class Sprintf extends FormattingFunction, NonThrowingFunction {
/**
* Implements `Snprintf`.
*/
private class SnprintfImpl extends Snprintf, AliasFunction, SideEffectFunction, NonThrowingFunction {
private class SnprintfImpl extends Snprintf, AliasFunction, SideEffectFunction,
NonCppThrowingFunction
{
SnprintfImpl() {
this instanceof TopLevelFunction and
(
Expand Down Expand Up @@ -205,7 +207,7 @@ private class StringCchPrintf extends FormattingFunction {
/**
* The standard function `syslog`.
*/
private class Syslog extends FormattingFunction, NonThrowingFunction {
private class Syslog extends FormattingFunction, NonCppThrowingFunction {
Syslog() {
this instanceof TopLevelFunction and
this.hasGlobalName("syslog") and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import semmle.code.cpp.models.interfaces.NonThrowing
* Does not include `strlcat`, which is covered by `StrlcatFunction`
*/
class StrcatFunction extends TaintFunction, DataFlowFunction, ArrayFunction, SideEffectFunction,
NonThrowingFunction
NonCppThrowingFunction
{
StrcatFunction() {
this.hasGlobalOrStdOrBslName([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ 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,
NonThrowingFunction
NonCppThrowingFunction
{
StrcpyFunction() {
this.hasGlobalOrStdOrBslName([
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import semmle.code.cpp.models.interfaces.Throwing

class WindowsDriverFunction extends ThrowingFunction {
WindowsDriverFunction() {
class WindowsDriverExceptionAnnotation extends AlwaysSehThrowingFunction {
WindowsDriverExceptionAnnotation() {
this.hasGlobalName(["RaiseException", "ExRaiseAccessViolation", "ExRaiseDatatypeMisalignment"])
}

final override predicate mayThrowException(boolean unconditional) { unconditional = true }
}
10 changes: 9 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/models/interfaces/NonThrowing.qll
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,15 @@
import semmle.code.cpp.Function
import semmle.code.cpp.models.Models

/**
* A function that is guaranteed to never throw a C++ exception
* (distinct from a structured exception handling, SEH, exception).
Comment on lines +9 to +10
Copy link
Contributor

@jketema jketema Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* A function that is guaranteed to never throw a C++ exception
* (distinct from a structured exception handling, SEH, exception).
* A function that is guaranteed to never throw a C++ exception.
*
* The function may still raise a structured exception handling (SEH) exception.

*/
abstract class NonCppThrowingFunction extends Function { }

/**
* A function that is guaranteed to never throw.
*
* DEPRECATED: use `NonCppThrowingFunction` instead.
*/
abstract class NonThrowingFunction extends Function { }
jketema marked this conversation as resolved.
Show resolved Hide resolved
deprecated class NonThrowingFunction = NonCppThrowingFunction;
11 changes: 10 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/models/interfaces/Throwing.qll
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,21 @@ import semmle.code.cpp.models.Models
import semmle.code.cpp.models.interfaces.FunctionInputsAndOutputs

/**
* A class that models the exceptional behavior of a function.
* A function that is known to raise an exception.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave this as it was, as we're going to deprecate this anyway.

*/
abstract class ThrowingFunction extends Function {
ThrowingFunction() { any() }

Comment on lines +17 to +18
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem necessary?

/**
* Holds if this function may throw an exception during evaluation.
* If `unconditional` is `true` the function always throws an exception.
*/
abstract predicate mayThrowException(boolean unconditional);
}

/**
* A function that is known to raise an exception unconditionally.
* The only cases known where this happens is for SEH
* (structured exception handling) exceptions.
*/
Comment on lines +26 to +30
Copy link
Contributor

@jketema jketema Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since "SEH" is in the name of the class, I would change this to just:

Suggested change
/**
* A function that is known to raise an exception unconditionally.
* The only cases known where this happens is for SEH
* (structured exception handling) exceptions.
*/
/**
* A function that unconditionally raises a structured exception handling (SEH) exception.
*/

abstract class AlwaysSehThrowingFunction extends Function { }
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ 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 f instanceof NonThrowingFunction and
not f instanceof NonCppThrowingFunction and
(not exists(f.getBlock()) or stmtMayThrow(f.getBlock()))
}

Expand Down