Skip to content

Commit c1207e0

Browse files
committed
Ruby: Fix rack response tracking
Use type tracking instead of getReturningNode, which seems to be faster and works correctly for the cases I've tried.
1 parent 21ce9b4 commit c1207e0

File tree

3 files changed

+33
-1
lines changed

3 files changed

+33
-1
lines changed

ruby/ql/lib/codeql/ruby/frameworks/Rack.qll

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ private import codeql.ruby.DataFlow
99
* Provides modeling for the Rack library.
1010
*/
1111
module Rack {
12+
import codeql.ruby.typetracking.TypeTracker
13+
1214
/**
1315
* A class that may be a rack application.
1416
* This is a class that has a `call` method that takes a single argument
@@ -20,7 +22,7 @@ module Rack {
2022
AppCandidate() {
2123
call = this.getInstanceMethod("call") and
2224
call.getNumberOfParameters() = 1 and
23-
exists(DataFlow::LocalSourceNode resp | isRackResponse(resp) | resp.flowsTo(call.getReturn()))
25+
call.getReturn() = trackRackResponse()
2426
}
2527

2628
/**
@@ -37,4 +39,15 @@ module Rack {
3739
exists(DataFlow::LocalSourceNode n | n.asExpr() = arr | n.flowsTo(r))
3840
)
3941
}
42+
43+
private DataFlow::LocalSourceNode trackRackResponse(TypeTracker t) {
44+
t.start() and
45+
isRackResponse(result)
46+
or
47+
exists(TypeTracker t2 | result = trackRackResponse(t2).track(t2, t))
48+
}
49+
50+
private DataFlow::Node trackRackResponse() {
51+
trackRackResponse(TypeTracker::end()).flowsTo(result)
52+
}
4053
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
| rack.rb:1:1:5:3 | HelloWorld | rack.rb:2:12:2:14 | env |
22
| rack.rb:7:1:16:3 | Proxy | rack.rb:12:12:12:18 | the_env |
33
| rack.rb:18:1:31:3 | Logger | rack.rb:24:12:24:14 | env |
4+
| rack.rb:45:1:61:3 | Baz | rack.rb:46:12:46:14 | env |

ruby/ql/test/library-tests/frameworks/rack/rack.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,3 +41,21 @@ def call(env)
4141
nil
4242
end
4343
end
44+
45+
class Baz
46+
def call(env)
47+
run(env)
48+
end
49+
50+
def run(env)
51+
if env[:foo] == "foo"
52+
[200, {}, "foo"]
53+
else
54+
error
55+
end
56+
end
57+
58+
def error
59+
[400, {}, "nope"]
60+
end
61+
end

0 commit comments

Comments
 (0)