Failing test to demonstrate problem with detecting regex match calls in Ruby #13748

izuzak · 2023-07-14T15:02:22Z

This PR just demonstrates a problem with the current implementation of IncompleteHostnameRegExp for Ruby. Specifically, it seems that the rule with report false positives for any X.match(Y) method call where Y is a String and X is any object with a match method.

In the example in this PR, we define a new class ClassWithMatch with a match method, and call it with:

obj = ClassWithMatch.new
obj.match("http://docs.github.com/")

Running tests then reports this:

Executing 1 tests in 1 directories.
Extracting test database in ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp.
Compiling queries in ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp.
Executing tests in ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp.
--- expected
+++ actual
@@ -15,7 +15,7 @@
 | tst-IncompleteHostnameRegExp.rb:19:14:19:30 | ^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:20:13:20:26 | "#{...}$" | here |
...
+| tst-IncompleteHostnameRegExp.rb:62:16:62:38 | http://docs.github.com/ | This regular expression has an unescaped '.' before 'github.com/', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:62:15:62:39 | "http://docs.github.com/" | here |
[1/1 comp 466ms eval 1.3s] FAILED(RESULT) ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref
0 tests passed; 1 tests failed:
  FAILED: ruby/ql/test/query-tests/security/cwe-020/IncompleteHostnameRegExp/IncompleteHostnameRegExp.qlref

Notice the reported problem:

| tst-IncompleteHostnameRegExp.rb:62:16:62:38 | http://docs.github.com/ 
| This regular expression has an unescaped '.' before 'github.com/', so it might
match more hosts than expected. | tst-IncompleteHostnameRegExp.rb:62:15:62:39 
| "http://docs.github.com/" | here |

So, the rule incorrectly thought that the string "http://docs.github.com/" was a regex being used for matching, likely because in Ruby's String class has a match method which takes a String parameter for defining the regex:

https://ruby-doc.org/3.2.2/String.html#method-i-match

Add failing test to demonstrate problem with detecting regex match calls

7dba475

github-actions bot added the Ruby label Jul 14, 2023

izuzak mentioned this pull request Jul 14, 2023

False positive for IncompleteHostnameRegExp in Ruby #13749

Open

Failing test to demonstrate problem with detecting regex match calls in Ruby #13748

Are you sure you want to change the base?

Failing test to demonstrate problem with detecting regex match calls in Ruby #13748

Conversation

izuzak commented Jul 14, 2023