Skip to content

Commit 642a134

Browse files
committed
add tests for the fixes in the qhelp, and fix an FP that appeared
1 parent 59c72b6 commit 642a134

File tree

3 files changed

+48
-0
lines changed

3 files changed

+48
-0
lines changed

ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -560,6 +560,11 @@ private predicate isArrayExpr(Expr e, ArrayLiteralCfgNode arr) {
560560
// Note(hmac): I don't think this is necessary, as `getSource` will not return
561561
// results if the source is a phi node.
562562
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isArrayConstant(n, arr))
563+
or
564+
// if `e` is an array, then `e.freeze` is also an array
565+
e instanceof MethodCall and
566+
e.(MethodCall).getMethodName() = "freeze" and
567+
isArrayExpr(e.(MethodCall).getReceiver(), arr)
563568
}
564569

565570
private class TokenConstantAccess extends ConstantAccess, TTokenConstantAccess {

ruby/ql/lib/codeql/ruby/security/UrlRedirectCustomizations.qll

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ module UrlRedirect {
8383
*/
8484
class StringConstCompareAsSanitizer extends Sanitizer, StringConstCompareBarrier { }
8585

86+
/**
87+
* A string concatenation against a constant list, considered as a sanitizer-guard.
88+
*/
89+
class StringConstArrayInclusionAsSanitizer extends Sanitizer, StringConstArrayInclusionCallBarrier
90+
{ }
91+
8692
/**
8793
* Some methods will propagate taint to their return values.
8894
* Here we cover a few common ones related to `ActionController::Parameters`.

ruby/ql/test/query-tests/security/cwe-601/UrlRedirect.rb

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,3 +98,40 @@ def filter_params(input_params)
9898
Rails.routes.draw do
9999
get "/r9", to: "users#route9"
100100
end
101+
102+
class DomainController < ActionController::Base
103+
KNOWN_HOST = "example.org"
104+
105+
def hello
106+
begin
107+
target_url = URI.parse(params[:url])
108+
109+
# Redirect if the URL is either relative or on a known good host
110+
if !target_url.host || target_url.host == KNOWN_HOST
111+
redirect_to target_url.to_s
112+
else
113+
redirect_to "/error.html" # Redirect to error page if the host is not known
114+
end
115+
rescue URI::InvalidURIError
116+
# Handle the exception, for example, by redirecting to a safe page
117+
redirect_to "/error.html"
118+
end
119+
end
120+
end
121+
122+
class ConstController < ActionController::Base
123+
VALID_REDIRECTS = [
124+
"http://cwe.mitre.org/data/definitions/601.html",
125+
"http://cwe.mitre.org/data/definitions/79.html"
126+
].freeze
127+
128+
def hello
129+
# GOOD: the request parameter is validated against a known list of URLs
130+
target_url = params[:url]
131+
if VALID_REDIRECTS.include?(target_url)
132+
redirect_to target_url
133+
else
134+
redirect_to "/error.html"
135+
end
136+
end
137+
end

0 commit comments

Comments
 (0)