A15-2-2: Avoid infinite interpretation edge case

In CodeQL CLI 2.12.7 there is a bug which causes an infinite loop
during results interpretation when a result includes more than maxPaths
paths and also includes a path with no edges i.e. where the source and
sink node are the same.

To avoid this edge case, if we report a path where the source and sink
are the same (i.e the throwingExpr directly throws an exception), we
adjust the sink node to report the constructor, which creates a one
step path from the throwingExprFlowNode to the constructor node.

This also means we can delete the `nodes` query predicate, as we only
included it to enable zero-path elements to display.
This commit is contained in:
Luke Cartey 2023-08-30 11:03:55 +01:00
Родитель afc027bea4
Коммит da1d12eb1d
3 изменённых файлов: 42 добавлений и 48 удалений

Просмотреть файл

@ -0,0 +1 @@
- `A15-2-2` - all results now include an associated exception flow path to avoid a CodeQL CLI bug in 2.12.7. This includes results where an exception is thrown directly in the constructor.

Просмотреть файл

@ -78,6 +78,14 @@ class DeleteWrapperFunction extends Function {
Parameter getADeleteParameter() { result = p }
}
class ExceptionThrowingConstructor extends ExceptionThrowingFunction, Constructor {
ExceptionThrowingConstructor() {
exists(getAFunctionThrownType(this, _)) and
// The constructor is within the users source code
exists(getFile().getRelativePath())
}
}
class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
Constructor c;
@ -90,24 +98,20 @@ class ExceptionThrownInConstructor extends ExceptionThrowingExpr {
Constructor getConstructor() { result = c }
}
/**
* Add the `nodes` predicate to ensure results with an empty path are still reported.
*/
query predicate nodes(ExceptionFlowNode node) { any() }
from
Constructor c, ExceptionThrownInConstructor throwingExpr, NewAllocationExpr newExpr,
ExceptionFlowNode exceptionSource, ExceptionFlowNode functionNode
ExceptionThrowingConstructor c, ExceptionThrownInConstructor throwingExpr,
NewAllocationExpr newExpr, ExceptionFlowNode exceptionSource,
ExceptionFlowNode throwingExprFlowNode, ExceptionFlowNode reportingNode
where
not isExcluded(c, Exceptions2Package::constructorErrorLeavesObjectInInvalidStateQuery()) and
not isNoExceptTrue(c) and
// Constructor must exit with an exception
c = throwingExpr.getConstructor() and
throwingExpr.hasExceptionFlowReflexive(exceptionSource, functionNode, _) and
throwingExpr.hasExceptionFlowReflexive(exceptionSource, throwingExprFlowNode, _) and
exists(ExceptionFlowNode mid |
edges*(exceptionSource, mid) and
newExpr.getASuccessor+() = mid.asThrowingExpr() and
edges*(mid, functionNode) and
edges*(mid, throwingExprFlowNode) and
not exists(ExceptionFlowNode prior | edges(prior, mid) |
prior.asCatchBlock().getEnclosingFunction() = c
)
@ -126,7 +130,16 @@ where
DataFlow::localFlow(DataFlow::exprNode(newExpr), DataFlow::exprNode(deletedExpr)) and
newExpr.getASuccessor+() = deletedExpr and
deletedExpr.getASuccessor+() = throwingExpr
)
select c, exceptionSource, functionNode, "Constructor throws $@ and allocates memory at $@",
) and
// In CodeQL CLI 2.12.7 there is a bug which causes an infinite loop during results interpretation
// when a result includes more than maxPaths paths and also includes a path with no edges i.e.
// where the source and sink node are the same.
// To avoid this edge case, if we report a path where the source and sink are the same (i.e the
// throwingExpr directly throws an exception), we adjust the sink node to report the constructor,
// which creates a one step path from the throwingExprFlowNode to the constructor node.
if throwingExprFlowNode = exceptionSource
then reportingNode.asFunction() = c and edges(throwingExprFlowNode, reportingNode)
else reportingNode = throwingExprFlowNode
select c, exceptionSource, reportingNode, "Constructor throws $@ and allocates memory at $@",
throwingExpr, throwingExpr.(ThrowingExpr).getAnExceptionType().getExceptionName(), newExpr,
"alloc"

Просмотреть файл

@ -3,60 +3,40 @@ edges
| test.cpp:13:7:13:28 | throw ... [exception] | test.cpp:14:33:16:5 | { ... } [exception] |
| test.cpp:14:33:16:5 | { ... } [bad_alloc] | test.cpp:15:7:15:11 | re-throw exception [bad_alloc] |
| test.cpp:14:33:16:5 | { ... } [exception] | test.cpp:15:7:15:11 | re-throw exception [exception] |
| test.cpp:15:7:15:11 | re-throw exception [bad_alloc] | test.cpp:9:3:9:8 | ClassA [bad_alloc] |
| test.cpp:15:7:15:11 | re-throw exception [exception] | test.cpp:9:3:9:8 | ClassA [exception] |
| test.cpp:25:16:25:27 | new [bad_alloc] | test.cpp:27:33:30:5 | { ... } [bad_alloc] |
| test.cpp:26:7:26:28 | throw ... [exception] | test.cpp:27:33:30:5 | { ... } [exception] |
| test.cpp:27:33:30:5 | { ... } [bad_alloc] | test.cpp:29:7:29:11 | re-throw exception [bad_alloc] |
| test.cpp:27:33:30:5 | { ... } [exception] | test.cpp:29:7:29:11 | re-throw exception [exception] |
| test.cpp:29:7:29:11 | re-throw exception [bad_alloc] | test.cpp:23:3:23:8 | ClassB [bad_alloc] |
| test.cpp:29:7:29:11 | re-throw exception [exception] | test.cpp:23:3:23:8 | ClassB [exception] |
| test.cpp:44:16:44:27 | call to CreateMember [bad_alloc] | test.cpp:46:33:48:5 | { ... } [bad_alloc] |
| test.cpp:45:7:45:28 | throw ... [exception] | test.cpp:46:33:48:5 | { ... } [exception] |
| test.cpp:46:33:48:5 | { ... } [bad_alloc] | test.cpp:47:7:47:11 | re-throw exception [bad_alloc] |
| test.cpp:46:33:48:5 | { ... } [exception] | test.cpp:47:7:47:11 | re-throw exception [exception] |
| test.cpp:47:7:47:11 | re-throw exception [bad_alloc] | test.cpp:41:3:41:8 | ClassC [bad_alloc] |
| test.cpp:47:7:47:11 | re-throw exception [exception] | test.cpp:41:3:41:8 | ClassC [exception] |
| test.cpp:58:16:58:27 | call to CreateMember [bad_alloc] | test.cpp:60:33:63:5 | { ... } [bad_alloc] |
| test.cpp:59:7:59:28 | throw ... [exception] | test.cpp:60:33:63:5 | { ... } [exception] |
| test.cpp:60:33:63:5 | { ... } [bad_alloc] | test.cpp:62:7:62:11 | re-throw exception [bad_alloc] |
| test.cpp:60:33:63:5 | { ... } [exception] | test.cpp:62:7:62:11 | re-throw exception [exception] |
| test.cpp:62:7:62:11 | re-throw exception [bad_alloc] | test.cpp:55:3:55:8 | ClassD [bad_alloc] |
| test.cpp:62:7:62:11 | re-throw exception [exception] | test.cpp:55:3:55:8 | ClassD [exception] |
| test.cpp:77:11:77:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] |
| test.cpp:78:11:78:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] |
| test.cpp:80:13:80:22 | new [bad_alloc] | test.cpp:82:33:86:5 | { ... } [bad_alloc] |
| test.cpp:81:13:81:22 | new [bad_alloc] | test.cpp:82:33:86:5 | { ... } [bad_alloc] |
| test.cpp:82:33:86:5 | { ... } [bad_alloc] | test.cpp:85:7:85:11 | re-throw exception [bad_alloc] |
nodes
| test.cpp:12:16:12:27 | new [bad_alloc] |
| test.cpp:13:7:13:28 | throw ... [exception] |
| test.cpp:14:33:16:5 | { ... } [bad_alloc] |
| test.cpp:14:33:16:5 | { ... } [exception] |
| test.cpp:15:7:15:11 | re-throw exception [bad_alloc] |
| test.cpp:15:7:15:11 | re-throw exception [exception] |
| test.cpp:25:16:25:27 | new [bad_alloc] |
| test.cpp:26:7:26:28 | throw ... [exception] |
| test.cpp:27:33:30:5 | { ... } [bad_alloc] |
| test.cpp:27:33:30:5 | { ... } [exception] |
| test.cpp:29:7:29:11 | re-throw exception [bad_alloc] |
| test.cpp:29:7:29:11 | re-throw exception [exception] |
| test.cpp:44:16:44:27 | call to CreateMember [bad_alloc] |
| test.cpp:45:7:45:28 | throw ... [exception] |
| test.cpp:46:33:48:5 | { ... } [bad_alloc] |
| test.cpp:46:33:48:5 | { ... } [exception] |
| test.cpp:47:7:47:11 | re-throw exception [bad_alloc] |
| test.cpp:47:7:47:11 | re-throw exception [exception] |
| test.cpp:58:16:58:27 | call to CreateMember [bad_alloc] |
| test.cpp:59:7:59:28 | throw ... [exception] |
| test.cpp:60:33:63:5 | { ... } [bad_alloc] |
| test.cpp:60:33:63:5 | { ... } [exception] |
| test.cpp:62:7:62:11 | re-throw exception [bad_alloc] |
| test.cpp:62:7:62:11 | re-throw exception [exception] |
| test.cpp:77:11:77:20 | new [bad_alloc] |
| test.cpp:78:11:78:20 | new [bad_alloc] |
| test.cpp:80:13:80:22 | new [bad_alloc] |
| test.cpp:81:13:81:22 | new [bad_alloc] |
| test.cpp:82:33:86:5 | { ... } [bad_alloc] |
| test.cpp:85:7:85:11 | re-throw exception [bad_alloc] |
| test.cpp:87:11:87:20 | new [bad_alloc] |
| test.cpp:85:7:85:11 | re-throw exception [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] |
| test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] |
#select
| test.cpp:9:3:9:8 | ClassA | test.cpp:13:7:13:28 | throw ... [exception] | test.cpp:15:7:15:11 | re-throw exception [exception] | Constructor throws $@ and allocates memory at $@ | test.cpp:15:7:15:11 | re-throw exception | std::bad_alloc | test.cpp:12:16:12:27 | new | alloc |
| test.cpp:9:3:9:8 | ClassA | test.cpp:13:7:13:28 | throw ... [exception] | test.cpp:15:7:15:11 | re-throw exception [exception] | Constructor throws $@ and allocates memory at $@ | test.cpp:15:7:15:11 | re-throw exception | std::exception | test.cpp:12:16:12:27 | new | alloc |
| test.cpp:41:3:41:8 | ClassC | test.cpp:45:7:45:28 | throw ... [exception] | test.cpp:47:7:47:11 | re-throw exception [exception] | Constructor throws $@ and allocates memory at $@ | test.cpp:47:7:47:11 | re-throw exception | std::bad_alloc | test.cpp:44:16:44:27 | call to CreateMember | alloc |
| test.cpp:41:3:41:8 | ClassC | test.cpp:45:7:45:28 | throw ... [exception] | test.cpp:47:7:47:11 | re-throw exception [exception] | Constructor throws $@ and allocates memory at $@ | test.cpp:47:7:47:11 | re-throw exception | std::exception | test.cpp:44:16:44:27 | call to CreateMember | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:78:11:78:20 | new [bad_alloc] | test.cpp:78:11:78:20 | new [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:78:11:78:20 | new | std::bad_alloc | test.cpp:77:11:77:20 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:87:11:87:20 | new [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:77:11:77:20 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:87:11:87:20 | new [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:78:11:78:20 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:87:11:87:20 | new [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:80:13:80:22 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:87:11:87:20 | new [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:81:13:81:22 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:78:11:78:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:78:11:78:20 | new | std::bad_alloc | test.cpp:77:11:77:20 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:77:11:77:20 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:78:11:78:20 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:80:13:80:22 | new | alloc |
| test.cpp:75:3:75:8 | ClassE | test.cpp:87:11:87:20 | new [bad_alloc] | test.cpp:75:3:75:8 | ClassE [bad_alloc] | Constructor throws $@ and allocates memory at $@ | test.cpp:87:11:87:20 | new | std::bad_alloc | test.cpp:81:13:81:22 | new | alloc |