Skip to content

Commit 51bc8e9

Browse files
committed
Ruby: Reduce FPs for rb/incomplete-hostname-regexp
Arguments in calls to `match[?]` should only be considered regular expression interpretations if the `match` refers to the standard library method, not a method in source code.
1 parent 8ccedd6 commit 51bc8e9

File tree

3 files changed

+18
-1
lines changed

3 files changed

+18
-1
lines changed

ruby/ql/lib/codeql/ruby/Regexp.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,9 @@ class StdLibRegExpInterpretation extends RegExpInterpretation::Range {
122122
mce.getMethodName() = ["match", "match?"] and
123123
this = mce.getArgument(0) and
124124
// exclude https://ruby-doc.org/core-2.4.0/Regexp.html#method-i-match
125-
not mce.getReceiver() = RegExpTracking::trackRegexpType()
125+
not mce.getReceiver() = RegExpTracking::trackRegexpType() and
126+
// exclude non-stdlib methods
127+
not exists(mce.getATarget())
126128
)
127129
}
128130
}

ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,4 @@
2828
| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.example\\.com/ | This string, which is used as a regular expression $@, has an unescaped '.' before 'example\\.com/', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:48:13:48:69 | ... + ... | here |
2929
| tst-IncompleteHostnameRegExp.rb:48:42:48:67 | ^https?://.+.example\\.com/ | This string, which is used as a regular expression $@, has an unrestricted wildcard '.+' which may cause 'example\\.com/' to be matched anywhere in the URL, outside the hostname. | tst-IncompleteHostnameRegExp.rb:48:13:48:69 | ... + ... | here |
3030
| tst-IncompleteHostnameRegExp.rb:59:5:59:20 | foo.example\\.com | This regular expression has an unescaped '.' before 'example\\.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:59:2:59:32 | /^(foo.example\\.com\|whatever)$/ | here |
31+
| tst-IncompleteHostnameRegExp.rb:81:11:81:34 | ^http://test.example.com | This string, which is used as a regular expression $@, has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:77:22:77:22 | x | here |

ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/tst-IncompleteHostnameRegExp.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,3 +65,17 @@ def convert1(domain)
6565
def convert2(domain)
6666
return Regexp.new(domain[:hostname]);
6767
end
68+
69+
class A
70+
def self.match?(x) = true
71+
end
72+
73+
A.match?("^http://test.example.com") # OK
74+
75+
class B
76+
def self.match?(x)
77+
some_string.match?(x)
78+
end
79+
end
80+
81+
B.match?("^http://test.example.com") # NOT OK

0 commit comments

Comments
 (0)