diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll index 9786286389c8..aadc53dd459b 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowNodes.qll @@ -29,11 +29,12 @@ private predicate deadcode(Expr e) { module SsaFlow { module Impl = SsaImpl::DataFlowIntegration; - private predicate ssaDefAssigns(SsaExplicitWrite def, Expr value) { + private predicate ssaDefAssigns(SsaExplicitWrite def, Node value) { exists(VariableUpdate upd | upd = def.getDefiningExpr() | - value = upd.(VariableAssign).getSource() or - value = upd.(AssignOp) or - value = upd.(RecordBindingVariableExpr) + value.asExpr() = upd.(VariableAssign).getSource() or + value.asExpr() = upd.(AssignOp) or + value.asExpr() = upd.(RecordBindingVariableExpr) or + value.(CatchParameterNode).getVariable() = upd ) } @@ -49,7 +50,7 @@ module SsaFlow { result.(Impl::WriteDefSourceNode).getDefinition().(SsaParameterInit).getParameter() = p ) or - ssaDefAssigns(result.(Impl::WriteDefSourceNode).getDefinition(), n.asExpr()) + ssaDefAssigns(result.(Impl::WriteDefSourceNode).getDefinition(), n) } predicate localFlowStep(SsaSourceVariable v, Node nodeFrom, Node nodeTo, boolean isUseStep) { @@ -99,6 +100,11 @@ private module Cached { TFlowSummaryNode(FlowSummaryImpl::Private::SummaryNode sn) or TFieldValueNode(Field f) or TCaptureNode(CaptureFlow::SynthesizedCaptureNode cn) or + TExceptionOutNode(DataFlowCall call) or + TExceptionReturnNode(DataFlowCallable callable) or + TCatchTypeTestNode(CatchClause catch) or + TCatchParameterNode(CatchClause catch) or + TUncaughtNode(TryStmt try) { ExceptionFlow::tryCatch(try, _) } or TAdditionalNode(Expr e, string id) { any(AdditionalDataFlowNode adfn).nodeAt(e, id) } cached @@ -177,6 +183,14 @@ module Public { or result = this.(FieldValueNode).getField().getType() or + result instanceof TypeException and this instanceof ExceptionOutNode + or + result instanceof TypeException and this instanceof CatchTypeTestNode + or + result = this.(CatchParameterNode).getVariable().getType() + or + result instanceof TypeException and this instanceof UncaughtNode + or result instanceof TypeObject and this instanceof AdditionalNode or result = this.(SsaNode).getTypeImpl() @@ -383,6 +397,27 @@ module Public { predicate isOwnInstanceAccess() { this.getInstanceAccess().isOwnInstanceAccess() } } + /** + * A node representing a thrown exception as the result of a call. + */ + class ExceptionOutNode extends Node, TExceptionOutNode { + override string toString() { result = "Exception out: " + this.getCall().toString() } + + override Location getLocation() { result = this.getCall().getLocation() } + + /** Gets the associated call. */ + DataFlowCall getCall() { this = TExceptionOutNode(result) } + } + + /** + * A node representing a thrown exception being returned from a callable. + */ + class ExceptionReturnNode extends Node, TExceptionReturnNode { + override string toString() { result = "Exception return" } + + override Location getLocation() { result = this.getEnclosingCallable().getLocation() } + } + /** A node introduced by an extension of `AdditionalDataFlowNode`. */ class AdditionalNode extends Node, TAdditionalNode { Expr e_; @@ -455,6 +490,11 @@ module Private { result.asSummarizedCallable() = n.(FlowSummaryNode).getSummarizedCallable() or result.asCallable() = n.(CaptureNode).getSynthesizedCaptureNode().getEnclosingCallable() or result.asFieldScope() = n.(FieldValueNode).getField() or + result = n.(ExceptionOutNode).getCall().getEnclosingCallable() or + n = TExceptionReturnNode(result) or + result.asCallable() = n.(CatchTypeTestNode).getCatch().getEnclosingCallable() or + result.asCallable() = n.(CatchParameterNode).getCatch().getEnclosingCallable() or + result.asCallable() = n.(UncaughtNode).getTry().getEnclosingCallable() or result.asCallable() = any(Expr e | n.(AdditionalNode).nodeAt(e, _)).getEnclosingCallable() or result.asCallable() = n.(SsaNode).getBasicBlock().getEnclosingCallable() } @@ -507,15 +547,23 @@ module Private { DataFlowCall getCall() { this.argumentOf(result, _) } } - /** A data flow node that occurs as the result of a `ReturnStmt`. */ + /** + * A data flow node that occurs as the result of a `ReturnStmt` or an + * exception being returned. + */ class ReturnNode extends Node { ReturnNode() { exists(ReturnStmt ret | this.asExpr() = ret.getResult()) or - this.(FlowSummaryNode).isReturn() + this.(FlowSummaryNode).isReturn() or + this instanceof ExceptionReturnNode } /** Gets the kind of this returned value. */ - ReturnKind getKind() { any() } + ReturnKind getKind() { + if this instanceof ExceptionReturnNode + then result instanceof ExceptionReturnKind + else result instanceof NormalReturnKind + } } /** A data flow node that represents the output of a call. */ @@ -524,6 +572,8 @@ module Private { this.asExpr() instanceof MethodCall or this.(FlowSummaryNode).isOut(_) + or + this instanceof ExceptionOutNode } /** Gets the underlying call. */ @@ -531,6 +581,15 @@ module Private { result.asCall() = this.asExpr() or this.(FlowSummaryNode).isOut(result) + or + result = this.(ExceptionOutNode).getCall() + } + + /** Gets the kind of this returned value. */ + ReturnKind getKind() { + if this instanceof ExceptionOutNode + then result instanceof ExceptionReturnKind + else result instanceof NormalReturnKind } } @@ -597,6 +656,67 @@ module Private { cn.isInstanceAccess() and result = cn.getEnclosingCallable().getDeclaringType() } } + + /** + * A data flow node that carries an exception and tests if it is caught in a + * given catch clause. + */ + class CatchTypeTestNode extends Node, TCatchTypeTestNode { + override string toString() { result = this.getCatch().toString() } + + override Location getLocation() { result = this.getCatch().getLocation() } + + /** Gets the catch clause associated with this node. */ + CatchClause getCatch() { this = TCatchTypeTestNode(result) } + + Node getSuccessor(boolean match) { + match = true and + this.getCatch() = result.(CatchParameterNode).getCatch() + or + match = false and + exists(TryStmt try, int i, CatchClause cc | + cc = this.getCatch() and + cc = try.getCatchClause(i) and + // A catch-all does not allow for uncaught exceptions. + not cc.getACaughtType() instanceof TypeThrowable and + not cc.getACaughtType() instanceof TypeException + | + result.(CatchTypeTestNode).getCatch() = try.getCatchClause(i + 1) + or + not exists(try.getCatchClause(i + 1)) and + result.(UncaughtNode).getTry() = try + ) + } + } + + /** + * A data flow node that holds the value of a variable defined in a catch + * clause. + */ + class CatchParameterNode extends Node, TCatchParameterNode { + override string toString() { result = this.getVariable().toString() } + + override Location getLocation() { result = this.getVariable().getLocation() } + + /** Gets the catch clause associated with this node. */ + CatchClause getCatch() { this = TCatchParameterNode(result) } + + /** Gets the variable declaration associated with this node. */ + LocalVariableDeclExpr getVariable() { result = this.getCatch().getVariable() } + } + + /** + * A data flow node that carries an exception that is uncaught by a try-catch + * statement. + */ + class UncaughtNode extends Node, TUncaughtNode { + override string toString() { result = "Uncaught exception" } + + override Location getLocation() { result = this.getTry().getLocation() } + + /** Gets the try statement associated with this node. */ + TryStmt getTry() { this = TUncaughtNode(result) } + } } private import Private diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll index 3dcdc1887614..847cd20b248a 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll @@ -17,15 +17,28 @@ private import DataFlowNodes private import codeql.dataflow.VariableCapture as VariableCapture import DataFlowNodes::Private -private newtype TReturnKind = TNormalReturnKind() +private newtype TReturnKind = + TNormalReturnKind() or + TExceptionReturnKind() /** - * A return kind. A return kind describes how a value can be returned - * from a callable. For Java, this is simply a method return. + * A return kind. A return kind describes how a value can be returned from a + * callable. For Java, this is either a normal method return or an exception + * being returned. */ class ReturnKind extends TReturnKind { /** Gets a textual representation of this return kind. */ - string toString() { result = "return" } + string toString() { none() } +} + +/** A return kind indicating normal method return. */ +class NormalReturnKind extends ReturnKind, TNormalReturnKind { + override string toString() { result = "return" } +} + +/** A return kind indicating exceptional method return. */ +class ExceptionReturnKind extends ReturnKind, TExceptionReturnKind { + override string toString() { result = "exception return" } } /** @@ -34,7 +47,7 @@ class ReturnKind extends TReturnKind { */ OutNode getAnOutNode(DataFlowCall call, ReturnKind kind) { result.getCall() = call and - kind = TNormalReturnKind() + result.getKind() = kind } /** @@ -170,6 +183,8 @@ private CaptureFlow::ClosureNode asClosureNode(Node n) { n.asExpr() = write.(VariableAssign).getSource() or n.asExpr() = write.(AssignOp) + or + n.(CatchParameterNode).getVariable() = write ) } @@ -203,6 +218,59 @@ predicate jumpStep(Node node1, Node node2) { node2.(FlowSummaryNode).getSummaryNode()) } +module ExceptionFlow { + /** + * Holds if `try` has at least one catch clause and `body` is either the main + * body of the `try` or one of its resource declarations. + */ + predicate tryCatch(TryStmt try, Stmt body) { + exists(try.getACatchClause()) and + ( + body = try.getBlock() or + body = try.getAResourceDecl() + ) + } + + /** + * Holds if `s2` is the enclosing statement of `s1` and `s1` is not directly + * wrapped in a try-catch. + */ + private predicate excStep(Stmt s1, Stmt s2) { + s1.getEnclosingStmt() = s2 and + not tryCatch(_, s1) + } + + pragma[nomagic] + private DataFlowCallable excReturnGetCallable(ExceptionReturnNode n) { + result = nodeGetEnclosingCallable(n) + } + + /** Holds if a thrown exception can flow locally from `node1` to `node2`. */ + predicate localStep(Node node1, Node node2) { + node1.(ExceptionOutNode).getCall().(SummaryCall).getEnclosingCallable() = + excReturnGetCallable(node2) + or + exists(Stmt exc | + node1.asExpr() = exc.(ThrowStmt).getExpr() or + node1.(ExceptionOutNode).getCall().asCall().getEnclosingStmt() = exc or + node1.(UncaughtNode).getTry() = exc + | + exists(TryStmt try, Stmt body | + excStep+(exc, body) and + tryCatch(try, body) and + node2.(CatchTypeTestNode).getCatch() = try.getCatchClause(0) + ) + or + exists(Callable callable | + excStep+(exc, callable.getBody()) and + excReturnGetCallable(node2).asCallable() = callable + ) + ) + or + node1.(CatchTypeTestNode).getSuccessor(_) = node2 + } +} + /** * Holds if `fa` is an access to an instance field that occurs as the * destination of an assignment of the value `src`. @@ -391,9 +459,9 @@ pragma[nomagic] predicate compatibleTypes(DataFlowType t1, DataFlowType t2) { erasedHaveIntersection(t1, t2) } /** A node that performs a type cast. */ -class CastNode extends ExprNode { +class CastNode extends Node { CastNode() { - this.getExpr() instanceof CastingExpr + this.asExpr() instanceof CastingExpr or exists(SsaExplicitWrite upd | upd.getDefiningExpr().(VariableAssign).getSource() = @@ -403,6 +471,8 @@ class CastNode extends ExprNode { ] and this.asExpr() = ssaGetAFirstUse(upd) ) + or + this instanceof CatchParameterNode } } diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll index 00e7d15ee8b8..ee5cad2fce46 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/DataFlowUtil.qll @@ -234,6 +234,8 @@ private predicate simpleLocalFlowStep0(Node node1, Node node2, string model) { simpleAstFlowStep(node1.asExpr(), node2.asExpr()) or captureValueStep(node1, node2) + or + ExceptionFlow::localStep(node1, node2) ) and model = "" or diff --git a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll index 8dc28ea2f601..8b73a431a67f 100644 --- a/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll +++ b/java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll @@ -48,7 +48,7 @@ module Input implements InputSig { ArgumentPosition callbackSelfParameterPosition() { result = -1 } - ReturnKind getStandardReturnValueKind() { any() } + ReturnKind getStandardReturnValueKind() { result instanceof NormalReturnKind } string encodeParameterPosition(ParameterPosition pos) { result = positionToString(pos) } @@ -117,8 +117,12 @@ private module TypesInput implements Impl::Private::TypesInputSig { } DataFlowType getReturnType(Impl::Public::SummarizedCallable c, ReturnKind rk) { - result = getErasedRepr(c.getReturnType()) and - exists(rk) + rk instanceof NormalReturnKind and + result = getErasedRepr(c.getReturnType()) + or + rk instanceof ExceptionReturnKind and + exists(c) and + result instanceof TypeThrowable } DataFlowType getCallbackParameterType(DataFlowType t, ArgumentPosition pos) { @@ -128,8 +132,12 @@ private module TypesInput implements Impl::Private::TypesInputSig { } DataFlowType getCallbackReturnType(DataFlowType t, ReturnKind rk) { - result = getErasedRepr(t.(FunctionalInterface).getRunMethod().getReturnType()) and - exists(rk) + rk instanceof NormalReturnKind and + result = getErasedRepr(t.(FunctionalInterface).getRunMethod().getReturnType()) + or + rk instanceof ExceptionReturnKind and + exists(t) and + result instanceof TypeThrowable } DataFlowType getSourceType(Input::SourceBase source, Impl::Private::SummaryComponentStack s) { @@ -384,7 +392,7 @@ module Private { SummaryComponent mapValue() { result = content(any(MapValueContent c)) } /** Gets a summary component that represents the return value of a call. */ - SummaryComponent return() { result = return(_) } + SummaryComponent return() { result = return(any(NormalReturnKind n)) } class SyntheticGlobal = Impl::Private::SyntheticGlobal; } diff --git a/java/ql/test/library-tests/dataflow/capture/B.java b/java/ql/test/library-tests/dataflow/capture/B.java index 6fc406c1a21c..40ecc38ae67b 100644 --- a/java/ql/test/library-tests/dataflow/capture/B.java +++ b/java/ql/test/library-tests/dataflow/capture/B.java @@ -279,4 +279,15 @@ String get() { sink(new MyLocal4().get()); // $ hasValueFlow=init sink(new MyLocal4(1).get()); // $ hasValueFlow=init } + + void testCapturedCatch() { + try { + throw new RuntimeException(source("rte")); + } catch (RuntimeException e) { + Runnable r = () -> { + sink(e.getMessage()); // $ hasValueFlow=rte + }; + r.run(); + } + } } diff --git a/java/ql/test/library-tests/dataflow/exceptions/A.java b/java/ql/test/library-tests/dataflow/exceptions/A.java new file mode 100644 index 000000000000..60840e84baa3 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/exceptions/A.java @@ -0,0 +1,92 @@ +import java.util.*; + +public class A { + static String source(String tag) { return tag; } + + static void sink(Object o) { } + + static class MyException extends RuntimeException { + String msg; + MyException(String msg) { + this.msg = msg; + } + } + + static class MySubException extends MyException { + MySubException(String msg) { + super(msg); + } + } + + static class MyOtherException extends RuntimeException { + String msg; + MyOtherException(String msg) { + this.msg = msg; + } + } + + void throwSome(int i) { + if (i == 1) throw new MyException(source("my")); + if (i == 2) throw new MySubException(source("sub")); + try { + if (i == 3) throw new MyOtherException(source("other")); + } catch (ClassCastException e) { + } + } + + void foo(int i) { + try { + try { + throwSome(i); + } finally { + } + } catch (MySubException e) { + sink(e.msg); // $ hasValueFlow=sub SPURIOUS: hasValueFlow=my + } catch (MyException e) { + sink(e.msg); // $ hasValueFlow=my SPURIOUS: hasValueFlow=sub + } catch (MyOtherException e) { + sink(e.msg); // $ hasValueFlow=other + } catch (Exception e) { + sink(((MyOtherException)e).msg); // $ SPURIOUS: hasValueFlow=other + } + } + + void throwArg(String msg) { + throw new MyException(msg); + } + + void catchSummary() { + try { + throwArg(source("arg")); + } catch (MyException e) { + sink(e.msg); // $ hasValueFlow=arg + } + } + + void runCallback(Runnable r) { + r.run(); + } + + void catchCallback() { + try { + runCallback(() -> { throw new MyException(source("cb")); }); + } catch (MyException e) { + sink(e.msg); // $ hasValueFlow=cb + } + + try { + List l = Arrays.asList(new String[] { "s" }); + l.forEach(s -> { throw new MyException(source("cb2")); }); + } catch (MyException e) { + sink(e.msg); // $ hasValueFlow=cb2 + } + } + + void catchRuntimeException() { + try { + throw new RuntimeException(source("rte")); + } catch (RuntimeException e) { + sink(e.getMessage()); // $ hasValueFlow=rte + } + } +} diff --git a/java/ql/test/library-tests/dataflow/exceptions/flow.ql b/java/ql/test/library-tests/dataflow/exceptions/flow.ql new file mode 100644 index 000000000000..abc1fbab9f87 --- /dev/null +++ b/java/ql/test/library-tests/dataflow/exceptions/flow.ql @@ -0,0 +1,3 @@ +import utils.test.InlineFlowTest +import DefaultFlowTest +import ValueFlow::PathGraph