diff --git a/python/ql/src/Exceptions/IllegalExceptionHandlerType.ql b/python/ql/src/Exceptions/IllegalExceptionHandlerType.ql index dc1c3f2fa35..e603abb64cb 100644 --- a/python/ql/src/Exceptions/IllegalExceptionHandlerType.ql +++ b/python/ql/src/Exceptions/IllegalExceptionHandlerType.ql @@ -12,20 +12,38 @@ */ import python -private import LegacyPointsTo +import semmle.python.dataflow.new.internal.DataFlowDispatch +private import ExceptionTypes -from ExceptFlowNodeWithPointsTo ex, Value t, ClassValue c, ControlFlowNode origin, string what +/** + * Gets an expression used as a handler type in the `except` clause at `ex`, + * either directly or as an element of a tuple. + */ +Expr handlerExpr(ExceptStmt ex) { + result = ex.getType() or + result = ex.getType().(Tuple).getAnElt() +} + +/** + * Gets an exception type used in the `except` clause at `ex`, + * where that type is not a legal exception type. + */ +ExceptType illegalHandlerType(ExceptStmt ex) { + result.getAUse().asExpr() = handlerExpr(ex) and + not result.isLegalExceptionType() +} + +from ExceptStmt ex, string msg where - ex.handledException(t, c, origin) and - ( - exists(ClassValue x | x = t | - not x.isLegalExceptionType() and - not x.failedInference(_) and - what = "class '" + x.getName() + "'" - ) - or - not t instanceof ClassValue and - what = "instance of '" + c.getName() + "'" + exists(ExceptType t | t = illegalHandlerType(ex) | + msg = + "Non-exception class '" + t.getName() + + "' in exception handler which will never match raised exception." ) -select ex.getNode(), - "Non-exception $@ in exception handler which will never match raised exception.", origin, what + or + exists(ImmutableLiteral lit | lit = handlerExpr(ex) and not lit instanceof None | + msg = + "Non-exception class '" + DuckTyping::getClassName(lit) + + "' in exception handler which will never match raised exception." + ) +select ex, msg diff --git a/python/ql/test/query-tests/Exceptions/general/IllegalExceptionHandlerType.expected b/python/ql/test/query-tests/Exceptions/general/IllegalExceptionHandlerType.expected index 5ba0c716371..f366f9de865 100644 --- a/python/ql/test/query-tests/Exceptions/general/IllegalExceptionHandlerType.expected +++ b/python/ql/test/query-tests/Exceptions/general/IllegalExceptionHandlerType.expected @@ -1,4 +1,3 @@ -| exceptions_test.py:51:5:51:25 | ExceptStmt | Non-exception $@ in exception handler which will never match raised exception. | exceptions_test.py:33:1:33:28 | ControlFlowNode for ClassExpr | class 'NotException1' | -| exceptions_test.py:54:5:54:25 | ExceptStmt | Non-exception $@ in exception handler which will never match raised exception. | exceptions_test.py:36:1:36:28 | ControlFlowNode for ClassExpr | class 'NotException2' | -| exceptions_test.py:138:5:138:22 | ExceptStmt | Non-exception $@ in exception handler which will never match raised exception. | exceptions_test.py:133:12:133:14 | ControlFlowNode for FloatLiteral | instance of 'float' | -| pypy_test.py:14:5:14:14 | ExceptStmt | Non-exception $@ in exception handler which will never match raised exception. | pypy_test.py:14:12:14:13 | ControlFlowNode for IntegerLiteral | instance of 'int' | +| exceptions_test.py:51:5:51:25 | ExceptStmt | Non-exception class 'NotException1' in exception handler which will never match raised exception. | +| exceptions_test.py:54:5:54:25 | ExceptStmt | Non-exception class 'NotException2' in exception handler which will never match raised exception. | +| pypy_test.py:14:5:14:14 | ExceptStmt | Non-exception class 'int' in exception handler which will never match raised exception. |