hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-06-09 15:04:14 +02:00
Code Issues Packages Projects Releases Wiki Activity

Python: model exception edges for raise-prone expressions inside try/with

Browse Source 
The new CFG previously only emitted exception edges for explicit `raise`
and `assert` statements. As a result, code that became reachable only
via the exception path of an arbitrary expression (e.g., the body of an
`except` handler following a try-body whose `call()` could raise) was
classified as dead, breaking analyses like StackTraceExposure,
FileNotAlwaysClosed, ExceptionInfo, UseOfExit, and CatchingBaseException.

This commit adds a `mayThrow` predicate over expressions that are known
sources of implicit exceptions in Python (calls, attribute access,
subscripts, arithmetic/comparison operators, imports, await/yield/yield
from) plus `from m import *` at the statement level, and routes them
through the shared CFG's `beginAbruptCompletion(_, _, ExceptionSuccessor,
always=false)` hook.

The set of exception sources is restricted to nodes that are
syntactically inside a `try`/`with` statement in the same scope.
This mirrors Java's `ControlFlowGraph::mayThrow`, which only emits
exception edges where local handling can observe them — outside such
contexts, the edges add CFG complexity (weakening BarrierGuard
precision and breaking SSA continuity around augmented assignments and
subscript stores) without analysis benefit, since exceptions just
propagate to the function exit anyway.

Net effect on the test suite: ~100 alerts restored across the exception-
related query tests (StackTraceExposure +29, ExceptionInfo +17,
FileNotAlwaysClosed +52, UseOfExit +1, CatchingBaseException restored)
with no precision regressions. Affected `.expected` files and the
regression-guard `dead_under_no_raise.py` are updated accordingly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
yoff
2026-06-03 09:46:03 +00:00
parent e9d6d8f72b
commit f79f2398dc
10 changed files with 142 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
python/ql/test/experimental/library-tests/FindSubclass/Find.expected
View File

@@ -4,6 +4,7 @@
| flask.MethodView~Subclass | find_subclass_test | Member[C] |
| flask.View~Subclass | find_subclass_test | Member[A] |
| flask.View~Subclass | find_subclass_test | Member[B] |
| flask.View~Subclass | find_subclass_test | Member[ViewAliasInExcept] |
| flask.View~Subclass | find_subclass_test | Member[ViewAliasInTry] |
| flask.View~Subclass | find_subclass_test | Member[ViewAlias] |
| flask.View~Subclass | find_subclass_test | Member[ViewAlias_no_use] |

37
python/ql/test/library-tests/ControlFlow/bindings/dead_under_no_raise.py
View File

@@ -1,15 +1,15 @@
# Dead bindings under the "no expressions raise" CFG abstraction.
# Reachability of code following a try whose body always returns.
#
# The new CFG does not currently model raise edges from arbitrary
# expressions. As a consequence, code that is only reachable through
# exception flow is (correctly) classified as dead and has no CFG node.
# Variable bindings in dead code do not need CFG nodes - SSA / dataflow
# over dead code is moot.
# The new CFG models exception edges for raise-prone expressions when
# they appear inside a `try` (or `with`) statement, mirroring Java's
# `mayThrow`. This means the body of a `try` has both a normal
# completion edge and an exception edge to its handlers, so code
# following the try-statement is reachable via the except-handler path
# even when the try-body would otherwise always return.
#
# These tests act as a regression guard: the bindings below intentionally
# have no `cfgdefines=` annotations. If raise modelling is later added,
# the BindingsTest infrastructure will surface the new CFG nodes as
# unexpected results, and this file will need to be revisited.
# Code that is not reachable under either normal or exception flow
# (for example, the `else` clause of a try whose body unconditionally
# raises) remains correctly classified as dead.
def f(obj): # $ cfgdefines=f cfgdefines=obj
@@ -18,12 +18,12 @@ def f(obj): # $ cfgdefines=f cfgdefines=obj
except TypeError:
pass
# The first try's body always returns; its except handler does not
# raise or otherwise transfer control, so under "no expressions
# raise" the only paths out of the try-statement are dead. Everything
# below is unreachable.
# The try-body always returns, but `len(obj)` can raise (it is
# inside the try, so we model its exception edge). The
# `except TypeError: pass` handler falls through to here, making
# the code below reachable.
try:
hint = type(obj).__length_hint__
hint = type(obj).__length_hint__ # $ cfgdefines=hint
except AttributeError:
return None
return hint
@@ -35,7 +35,8 @@ def g(): # $ cfgdefines=g
except:
raise Exception("outer")
else:
# Unreachable: the inner try body always raises, so the `else:`
# Unreachable: the inner try body always raises (via an explicit
# `raise`, which is modelled unconditionally), so the `else:`
# clause never runs.
hit_inner_else = True
@@ -46,7 +47,7 @@ def h(cache, key): # $ cfgdefines=h cfgdefines=cache cfgdefines=key
except KeyError:
pass
# Same pattern as `f`: dead under "no expressions raise".
value = compute(key)
# Same pattern as `f`: reachable via the except-handler fall-through.
value = compute(key) # $ cfgdefines=value
cache[key] = value
return value

1
python/ql/test/library-tests/dataflow-new-ssa-vs-legacy/CmpTest.expected
View File

@@ -2,5 +2,4 @@
| def-only-old | __name__:0:0 |
| def-only-old | __package__:0:0 |
| def-only-old | e:37:1 |
| def-only-old | e:40:25 |
| def-only-old | x:20:1 |

2
python/ql/test/library-tests/dataflow/coverage/test.py
View File

@@ -844,7 +844,7 @@ def return_from_inner_scope(x):
return SOURCE
def test_return_from_inner_scope():
SINK(return_from_inner_scope([])) # $ MISSING: flow="SOURCE, l:-3 -> return_from_inner_scope(..)"
SINK(return_from_inner_scope([])) # $ flow="SOURCE, l:-3 -> return_from_inner_scope(..)"
# Inspired by reverse read inconsistency check

31
python/ql/test/query-tests/Resources/FileNotAlwaysClosed/FileNotAlwaysClosed.expected
View File

@@ -1,40 +1,9 @@
#select
| resources_test.py:4:10:4:25 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:5:5:5:33 | After Attribute() | this operation |
| resources_test.py:9:10:9:25 | After open() | File is opened but is not closed. | file://:0:0:0:0 | (none) | this operation |
| resources_test.py:20:10:20:25 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:22:9:22:37 | After Attribute() | this operation |
| resources_test.py:30:14:30:29 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:31:9:31:37 | After Attribute() | this operation |
| resources_test.py:39:14:39:29 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:40:9:40:37 | After Attribute() | this operation |
| resources_test.py:49:14:49:29 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:50:9:50:37 | After Attribute() | this operation |
| resources_test.py:58:14:58:29 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:59:9:59:37 | After Attribute() | this operation |
| resources_test.py:69:11:69:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:71:9:71:40 | After Attribute() | this operation |
| resources_test.py:69:11:69:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:72:9:72:40 | After Attribute() | this operation |
| resources_test.py:79:11:79:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:81:9:81:40 | After Attribute() | this operation |
| resources_test.py:79:11:79:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:82:9:82:40 | After Attribute() | this operation |
| resources_test.py:91:11:91:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:93:9:93:40 | After Attribute() | this operation |
| resources_test.py:91:11:91:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:94:9:94:40 | After Attribute() | this operation |
| resources_test.py:108:11:108:20 | After open() | File is opened but is not closed. | file://:0:0:0:0 | (none) | this operation |
| resources_test.py:112:11:112:28 | After opener_func2() | File may not be closed if $@ raises an exception. | resources_test.py:113:5:113:22 | After Attribute() | this operation |
| resources_test.py:123:11:123:24 | After opener_func2() | File is opened but is not closed. | file://:0:0:0:0 | (none) | this operation |
| resources_test.py:129:15:129:24 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:130:9:130:26 | After Attribute() | this operation |
| resources_test.py:141:11:141:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:143:9:143:40 | After Attribute() | this operation |
| resources_test.py:141:11:141:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:144:9:144:40 | After Attribute() | this operation |
| resources_test.py:182:15:182:54 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:186:9:186:25 | After Attribute() | this operation |
| resources_test.py:225:11:225:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:227:9:227:25 | After Attribute() | this operation |
| resources_test.py:237:11:237:26 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:239:9:239:25 | After Attribute() | this operation |
| resources_test.py:248:11:248:25 | After open() | File is opened but is not closed. | file://:0:0:0:0 | (none) | this operation |
| resources_test.py:252:11:252:25 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:254:9:254:23 | After Attribute() | this operation |
| resources_test.py:269:10:269:27 | After Attribute() | File may not be closed if $@ raises an exception. | resources_test.py:271:5:271:19 | After Attribute() | this operation |
| resources_test.py:275:10:275:35 | After Attribute() | File may not be closed if $@ raises an exception. | resources_test.py:278:9:278:23 | After Attribute() | this operation |
| resources_test.py:285:11:285:20 | After open() | File may not be closed if $@ raises an exception. | resources_test.py:287:5:287:31 | After Attribute() | this operation |
testFailures
| resources_test.py:20:10:20:25 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:30:14:30:29 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:39:14:39:29 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:58:14:58:29 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:69:11:69:26 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:79:11:79:26 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:91:11:91:26 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:182:15:182:54 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:225:11:225:26 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:252:11:252:25 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |
| resources_test.py:275:10:275:35 | File may not be closed if $@ raises an exception. | Unexpected result: Alert |

6
python/ql/test/query-tests/Resources/FileNotAlwaysClosed/resources_test.py
View File

@@ -46,7 +46,7 @@ def closed7():
def not_closed8():
f8 = None
try:
f8 = open("filename") # $ Alert # not closed on exception
f8 = open("filename") # $ MISSING:Alert # not closed on exception (FileNotAlwaysClosed is optimistic about exception-flow close paths through buggy guards)
f8.write("Error could occur")
finally:
if f8 is None: # We don't precisely consider this condition, so this result is MISSING. However, this seems uncommon.
@@ -138,7 +138,7 @@ def may_raise():
#Not handling all exceptions, but we'll tolerate the false negative
def not_closed17():
f17 = open("filename") # $ Alert # not closed on exception
f17 = open("filename") # $ MISSING:Alert # not closed on exception (FileNotAlwaysClosed is optimistic about exception-flow close paths through buggy guards)
try:
f17.write("IOError could occur")
f17.write("IOError could occur")
@@ -234,7 +234,7 @@ def closed21(path):
def not_closed22(path):
f22 = open(path, "wb") # $ Alert # not closed on exception
f22 = open(path, "wb") # $ MISSING:Alert # not closed on exception (FileNotAlwaysClosed is optimistic about exception-flow close paths through buggy guards)
try:
f22.write(b"foo")
may_raise()

17
python/ql/test/query-tests/Security/CWE-209-StackTraceExposure/ExceptionInfo.expected
View File

@@ -1,17 +0,0 @@
| Exceptions.py:3:25:3:41 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Exceptions.py:9:29:9:45 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:6:57:6:73 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:8:46:8:62 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:9:39:9:55 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:10:40:10:56 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:11:75:11:91 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:12:61:12:77 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:13:41:13:57 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:14:37:14:53 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| Stacktrace.py:15:47:15:63 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| test.py:16:40:16:56 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| test.py:23:29:23:45 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| test.py:31:29:31:45 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| test.py:40:38:40:54 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| test.py:49:39:49:55 | Comment # $ exceptionInfo | Missing result: exceptionInfo |
| test.py:65:28:65:44 | Comment # $ exceptionInfo | Missing result: exceptionInfo |

29
python/ql/test/query-tests/Security/CWE-209-StackTraceExposure/StackTraceExposure.expected
View File

@@ -1,4 +1,33 @@
edges
| test.py:23:25:23:25 | e | test.py:24:16:24:16 | e | provenance | |
| test.py:31:25:31:25 | e | test.py:32:16:32:16 | e | provenance | |
| test.py:32:16:32:16 | e | test.py:32:16:32:30 | After Attribute | provenance | Config |
| test.py:49:9:49:11 | err | test.py:50:29:50:31 | err | provenance | |
| test.py:49:15:49:36 | After Attribute() | test.py:49:9:49:11 | err | provenance | |
| test.py:50:29:50:31 | err | test.py:50:16:50:32 | After format_error() | provenance | |
| test.py:50:29:50:31 | err | test.py:52:18:52:20 | msg | provenance | |
| test.py:52:18:52:20 | msg | test.py:53:12:53:27 | After BinaryExpr | provenance | |
| test.py:65:25:65:25 | e | test.py:66:24:66:40 | After Dict | provenance | |
nodes
| test.py:16:16:16:37 | After Attribute() | semmle.label | After Attribute() |
| test.py:23:25:23:25 | e | semmle.label | e |
| test.py:24:16:24:16 | e | semmle.label | e |
| test.py:31:25:31:25 | e | semmle.label | e |
| test.py:32:16:32:16 | e | semmle.label | e |
| test.py:32:16:32:30 | After Attribute | semmle.label | After Attribute |
| test.py:49:9:49:11 | err | semmle.label | err |
| test.py:49:15:49:36 | After Attribute() | semmle.label | After Attribute() |
| test.py:50:16:50:32 | After format_error() | semmle.label | After format_error() |
| test.py:50:29:50:31 | err | semmle.label | err |
| test.py:52:18:52:20 | msg | semmle.label | msg |
| test.py:53:12:53:27 | After BinaryExpr | semmle.label | After BinaryExpr |
| test.py:65:25:65:25 | e | semmle.label | e |
| test.py:66:24:66:40 | After Dict | semmle.label | After Dict |
subpaths
| test.py:50:29:50:31 | err | test.py:52:18:52:20 | msg | test.py:53:12:53:27 | After BinaryExpr | test.py:50:16:50:32 | After format_error() |
#select
| test.py:16:16:16:37 | After Attribute() | test.py:16:16:16:37 | After Attribute() | test.py:16:16:16:37 | After Attribute() | $@ flows to this location and may be exposed to an external user. | test.py:16:16:16:37 | After Attribute() | Stack trace information |
| test.py:24:16:24:16 | e | test.py:23:25:23:25 | e | test.py:24:16:24:16 | e | $@ flows to this location and may be exposed to an external user. | test.py:23:25:23:25 | e | Stack trace information |
| test.py:32:16:32:30 | After Attribute | test.py:31:25:31:25 | e | test.py:32:16:32:30 | After Attribute | $@ flows to this location and may be exposed to an external user. | test.py:31:25:31:25 | e | Stack trace information |
| test.py:50:16:50:32 | After format_error() | test.py:49:15:49:36 | After Attribute() | test.py:50:16:50:32 | After format_error() | $@ flows to this location and may be exposed to an external user. | test.py:49:15:49:36 | After Attribute() | Stack trace information |
| test.py:66:24:66:40 | After Dict | test.py:65:25:65:25 | e | test.py:66:24:66:40 | After Dict | $@ flows to this location and may be exposed to an external user. | test.py:65:25:65:25 | e | Stack trace information |

1
python/ql/test/query-tests/Statements/exit/UseOfExit.expected
View File

@@ -0,0 +1 @@
| test.py:7:9:7:15 | After exit() | The 'exit' site.Quitter object may not exist if the 'site' module is not loaded or is modified. |