diff --git a/ruby/ql/lib/codeql/ruby/frameworks/Rack.qll b/ruby/ql/lib/codeql/ruby/frameworks/Rack.qll index ec3673207ac..503af8e8533 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/Rack.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/Rack.qll @@ -2,6 +2,7 @@ * Provides modeling for the Rack library. */ +private import codeql.ruby.Concepts private import codeql.ruby.controlflow.CfgNodes::ExprNodes private import codeql.ruby.DataFlow private import codeql.ruby.typetracking.TypeTracker @@ -17,48 +18,63 @@ module Rack { */ class AppCandidate extends DataFlow::ClassNode { private DataFlow::MethodNode call; + private PotentialResponseNode resp; AppCandidate() { call = this.getInstanceMethod("call") and call.getNumberOfParameters() = 1 and - call.getReturn() = trackRackResponse() + call.getReturn() = trackRackResponse(resp) } /** * Gets the environment of the request, which is the lone parameter to the `call` method. */ DataFlow::ParameterNode getEnv() { result = call.getParameter(0) } + + /** Gets the response returned from the request. */ + PotentialResponseNode getResponse() { result = resp } } - private DataFlow::LocalSourceNode trackStatusCode(TypeTracker t, int i) { + private DataFlow::LocalSourceNode trackInt(TypeTracker t, int i) { t.start() and result.getConstantValue().isInt(i) or - exists(TypeTracker t2 | result = trackStatusCode(t2, i).track(t2, t)) + exists(TypeTracker t2 | result = trackInt(t2, i).track(t2, t)) } - private DataFlow::Node trackStatusCode(int i) { - trackStatusCode(TypeTracker::end(), i).flowsTo(result) - } + private DataFlow::Node trackInt(int i) { trackInt(TypeTracker::end(), i).flowsTo(result) } - class ResponseNode extends DataFlow::ArrayLiteralNode { + private class PotentialResponseNode extends DataFlow::ArrayLiteralNode { // [status, headers, body] - ResponseNode() { this.getNumberOfArguments() = 3 } + PotentialResponseNode() { this.getNumberOfArguments() = 3 } /** * Gets an HTTP status code that may be returned in this response. */ - int getAStatusCode() { this.getElement(0) = trackStatusCode(result) } + int getAStatusCode() { this.getElement(0) = trackInt(result) } } - private DataFlow::LocalSourceNode trackRackResponse(TypeTracker t) { + private DataFlow::LocalSourceNode trackRackResponse(TypeTracker t, PotentialResponseNode n) { t.start() and - result instanceof ResponseNode + result = n or - exists(TypeTracker t2 | result = trackRackResponse(t2).track(t2, t)) + exists(TypeTracker t2 | result = trackRackResponse(t2, n).track(t2, t)) } - private DataFlow::Node trackRackResponse() { - trackRackResponse(TypeTracker::end()).flowsTo(result) + private DataFlow::Node trackRackResponse(PotentialResponseNode n) { + trackRackResponse(TypeTracker::end(), n).flowsTo(result) + } + + /** A `DataFlow::Node` returned from a rack request. */ + class ResponseNode extends PotentialResponseNode, Http::Server::HttpResponse::Range { + ResponseNode() { this = any(AppCandidate app).getResponse() } + + override DataFlow::Node getBody() { result = this.getElement(2) } + + // TODO + override DataFlow::Node getMimetypeOrContentTypeArg() { none() } + + // TODO + override string getMimetypeDefault() { none() } } } diff --git a/ruby/ql/test/library-tests/frameworks/rack/Rack.expected b/ruby/ql/test/library-tests/frameworks/rack/Rack.expected index db76f4545b6..23fec9a8266 100644 --- a/ruby/ql/test/library-tests/frameworks/rack/Rack.expected +++ b/ruby/ql/test/library-tests/frameworks/rack/Rack.expected @@ -6,6 +6,7 @@ rackApps rackResponseStatusCodes | rack.rb:7:5:7:63 | call to [] | 200 | | rack.rb:7:5:7:63 | call to [] | 500 | -| rack.rb:39:5:39:13 | call to [] | 1 | +| rack.rb:18:5:18:27 | call to [] | | +| rack.rb:33:5:33:26 | call to [] | | | rack.rb:56:7:56:22 | call to [] | 200 | | rack.rb:63:5:63:21 | call to [] | 400 | diff --git a/ruby/ql/test/library-tests/frameworks/rack/Rack.ql b/ruby/ql/test/library-tests/frameworks/rack/Rack.ql index 25c8303853d..6e18162a5e1 100644 --- a/ruby/ql/test/library-tests/frameworks/rack/Rack.ql +++ b/ruby/ql/test/library-tests/frameworks/rack/Rack.ql @@ -3,6 +3,8 @@ private import codeql.ruby.DataFlow query predicate rackApps(Rack::AppCandidate c, DataFlow::ParameterNode env) { env = c.getEnv() } -query predicate rackResponseStatusCodes(Rack::ResponseNode resp, int status) { - status = resp.getAStatusCode() +query predicate rackResponseStatusCodes(Rack::ResponseNode resp, string status) { + if exists(resp.getAStatusCode()) + then status = resp.getAStatusCode().toString() + else status = "" }