Skip to content

Commit fb8cc6e

Browse files
aibaarsaibaars
committed
Ruby: String.index method returns 'nil', not '-1'
1 parent f2ec513 commit fb8cc6e

File tree

3 files changed

+54
-51
lines changed

3 files changed

+54
-51
lines changed

‎ruby/ql/lib/codeql/ruby/InclusionTests.qll

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ private import codeql.ruby.controlflow.CfgNodes
1313
* Examples:
1414
* ```
1515
* A.include?(B)
16-
* A.index(B) !== -1
16+
* A.index(B) != nil
1717
* A.index(B) >= 0
1818
* ```
1919
*/
@@ -74,7 +74,7 @@ module InclusionTest {
7474
}
7575

7676
/**
77-
* A check of form `A.index(B) != -1`, `A.index(B) >= 0`, or similar.
77+
* A check of form `A.index(B) != nil`, `A.index(B) >= 0`, or similar.
7878
*/
7979
private class Includes_IndexOfComparison extends Range, DataFlow::Node {
8080
private DataFlow::CallNode indexOf;
@@ -88,8 +88,11 @@ module InclusionTest {
8888
// one operand is of the form `whitelist.index(x)`
8989
indexOf.getMethodName() = "index" and
9090
// and the other one is 0 or -1
91-
value = index.getConstantValue().getInt() and
92-
value = [0, -1]
91+
(
92+
value = index.getConstantValue().getInt() and value = 0
93+
or
94+
index.getExpr() instanceof NilLiteral and value = -1
95+
)
9396
|
9497
value = -1 and polarity = false and comparison.getExpr() instanceof CaseEqExpr
9598
or

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,23 @@
1-
| tst-IncompleteUrlSubstringSanitization.rb:4:5:4:31 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:4:13:4:24 | "secure.com" | secure.com |
2-
| tst-IncompleteUrlSubstringSanitization.rb:5:5:5:31 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:5:13:5:24 | "secure.net" | secure.net |
3-
| tst-IncompleteUrlSubstringSanitization.rb:6:5:6:32 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:6:13:6:25 | ".secure.com" | .secure.com |
4-
| tst-IncompleteUrlSubstringSanitization.rb:10:5:10:32 | ... === ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:10:13:10:24 | "secure.com" | secure.com |
1+
| tst-IncompleteUrlSubstringSanitization.rb:4:5:4:32 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:4:13:4:24 | "secure.com" | secure.com |
2+
| tst-IncompleteUrlSubstringSanitization.rb:5:5:5:32 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:5:13:5:24 | "secure.net" | secure.net |
3+
| tst-IncompleteUrlSubstringSanitization.rb:6:5:6:33 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:6:13:6:25 | ".secure.com" | .secure.com |
4+
| tst-IncompleteUrlSubstringSanitization.rb:10:5:10:33 | ... === ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:10:13:10:24 | "secure.com" | secure.com |
55
| tst-IncompleteUrlSubstringSanitization.rb:11:5:11:31 | ... === ... | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.rb:11:13:11:24 | "secure.com" | secure.com |
66
| tst-IncompleteUrlSubstringSanitization.rb:12:5:12:30 | ... >= ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:12:13:12:24 | "secure.com" | secure.com |
77
| tst-IncompleteUrlSubstringSanitization.rb:14:5:14:39 | call to start_with? | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.rb:14:19:14:38 | "https://secure.com" | https://secure.com |
88
| tst-IncompleteUrlSubstringSanitization.rb:15:5:15:29 | call to end_with? | '$@' may be preceded by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.rb:15:17:15:28 | "secure.com" | secure.com |
99
| tst-IncompleteUrlSubstringSanitization.rb:20:5:20:28 | call to include? | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:20:16:20:27 | "secure.com" | secure.com |
10-
| tst-IncompleteUrlSubstringSanitization.rb:32:5:32:39 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:32:13:32:32 | "https://secure.com" | https://secure.com |
11-
| tst-IncompleteUrlSubstringSanitization.rb:33:5:33:43 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:33:13:33:36 | "https://secure.com:443" | https://secure.com:443 |
12-
| tst-IncompleteUrlSubstringSanitization.rb:34:5:34:40 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:34:13:34:33 | "https://secure.com/" | https://secure.com/ |
13-
| tst-IncompleteUrlSubstringSanitization.rb:52:5:52:45 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:52:13:52:38 | "https://example.internal" | https://example.internal |
10+
| tst-IncompleteUrlSubstringSanitization.rb:32:5:32:40 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:32:13:32:32 | "https://secure.com" | https://secure.com |
11+
| tst-IncompleteUrlSubstringSanitization.rb:33:5:33:44 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:33:13:33:36 | "https://secure.com:443" | https://secure.com:443 |
12+
| tst-IncompleteUrlSubstringSanitization.rb:34:5:34:41 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:34:13:34:33 | "https://secure.com/" | https://secure.com/ |
13+
| tst-IncompleteUrlSubstringSanitization.rb:52:5:52:46 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:52:13:52:38 | "https://example.internal" | https://example.internal |
1414
| tst-IncompleteUrlSubstringSanitization.rb:55:5:55:45 | call to start_with? | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.rb:55:19:55:44 | "https://example.internal" | https://example.internal |
1515
| tst-IncompleteUrlSubstringSanitization.rb:56:5:56:48 | ... != ... | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.rb:56:13:56:42 | "https://example.internal.org" | https://example.internal.org |
1616
| tst-IncompleteUrlSubstringSanitization.rb:57:5:57:49 | ... === ... | '$@' may be followed by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.rb:57:13:57:42 | "https://example.internal.org" | https://example.internal.org |
1717
| tst-IncompleteUrlSubstringSanitization.rb:58:5:58:31 | call to end_with? | '$@' may be preceded by an arbitrary host name. | tst-IncompleteUrlSubstringSanitization.rb:58:17:58:30 | "internal.com" | internal.com |
18-
| tst-IncompleteUrlSubstringSanitization.rb:61:2:61:28 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:61:10:61:21 | "secure.com" | secure.com |
19-
| tst-IncompleteUrlSubstringSanitization.rb:62:2:62:29 | ... === ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:62:10:62:21 | "secure.com" | secure.com |
20-
| tst-IncompleteUrlSubstringSanitization.rb:63:4:63:30 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:63:12:63:23 | "secure.com" | secure.com |
18+
| tst-IncompleteUrlSubstringSanitization.rb:61:2:61:29 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:61:10:61:21 | "secure.com" | secure.com |
19+
| tst-IncompleteUrlSubstringSanitization.rb:62:2:62:30 | ... === ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:62:10:62:21 | "secure.com" | secure.com |
20+
| tst-IncompleteUrlSubstringSanitization.rb:63:4:63:31 | ... != ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:63:12:63:23 | "secure.com" | secure.com |
2121
| tst-IncompleteUrlSubstringSanitization.rb:64:3:64:26 | call to include? | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:64:14:64:25 | "secure.com" | secure.com |
2222
| tst-IncompleteUrlSubstringSanitization.rb:66:6:66:29 | call to include? | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:66:17:66:28 | "secure.com" | secure.com |
2323
| tst-IncompleteUrlSubstringSanitization.rb:73:5:73:46 | ... >= ... | '$@' can be anywhere in the URL, and arbitrary hosts may come before or after it. | tst-IncompleteUrlSubstringSanitization.rb:73:13:73:40 | "https://secure.com/foo/bar" | https://secure.com/foo/bar |

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

Lines changed: 36 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
def test (x)
2-
x.index("internal") != -1; # NOT OK, but not flagged
3-
x.index("localhost") != -1; # NOT OK, but not flagged
4-
x.index("secure.com") != -1; # NOT OK
5-
x.index("secure.net") != -1; # NOT OK
6-
x.index(".secure.com") != -1; # NOT OK
7-
x.index("sub.secure.") != -1; # NOT OK, but not flagged
8-
x.index(".sub.secure.") != -1; # NOT OK, but not flagged
2+
x.index("internal") != nil; # NOT OK, but not flagged
3+
x.index("localhost") != nil; # NOT OK, but not flagged
4+
x.index("secure.com") != nil; # NOT OK
5+
x.index("secure.net") != nil; # NOT OK
6+
x.index(".secure.com") != nil; # NOT OK
7+
x.index("sub.secure.") != nil; # NOT OK, but not flagged
8+
x.index(".sub.secure.") != nil; # NOT OK, but not flagged
99

10-
x.index("secure.com") === -1; # NOT OK
10+
x.index("secure.com") === nil; # NOT OK
1111
x.index("secure.com") === 0; # NOT OK
1212
x.index("secure.com") >= 0; # NOT OK
1313

@@ -19,48 +19,48 @@ def test (x)
1919

2020
x.include?("secure.com"); # NOT OK
2121

22-
x.index("#") != -1; # OK
23-
x.index(":") != -1; # OK
24-
x.index(":/") != -1; # OK
25-
x.index("://") != -1; # OK
26-
x.index("//") != -1; # OK
27-
x.index(":443") != -1; # OK
28-
x.index("/some/path/") != -1; # OK
29-
x.index("some/path") != -1; # OK
30-
x.index("/index.html") != -1; # OK
31-
x.index(":template:") != -1; # OK
32-
x.index("https://secure.com") != -1; # NOT OK
33-
x.index("https://secure.com:443") != -1; # NOT OK
34-
x.index("https://secure.com/") != -1; # NOT OK
22+
x.index("#") != nil; # OK
23+
x.index(":") != nil; # OK
24+
x.index(":/") != nil; # OK
25+
x.index("://") != nil; # OK
26+
x.index("//") != nil; # OK
27+
x.index(":443") != nil; # OK
28+
x.index("/some/path/") != nil; # OK
29+
x.index("some/path") != nil; # OK
30+
x.index("/index.html") != nil; # OK
31+
x.index(":template:") != nil; # OK
32+
x.index("https://secure.com") != nil; # NOT OK
33+
x.index("https://secure.com:443") != nil; # NOT OK
34+
x.index("https://secure.com/") != nil; # NOT OK
3535

36-
x.index(".cn") != -1; # NOT OK, but not flagged
37-
x.index(".jpg") != -1; # OK
38-
x.index("index.html") != -1; # OK
39-
x.index("index.js") != -1; # OK
40-
x.index("index.php") != -1; # OK
41-
x.index("index.css") != -1; # OK
36+
x.index(".cn") != nil; # NOT OK, but not flagged
37+
x.index(".jpg") != nil; # OK
38+
x.index("index.html") != nil; # OK
39+
x.index("index.js") != nil; # OK
40+
x.index("index.php") != nil; # OK
41+
x.index("index.css") != nil; # OK
4242

43-
x.index("secure=true") != -1; # OK (query param)
44-
x.index("&auth=") != -1; # OK (query param)
43+
x.index("secure=true") != nil; # OK (query param)
44+
x.index("&auth=") != nil; # OK (query param)
4545

46-
x.index(getCurrentDomain()) != -1; # NOT OK, but not flagged
47-
x.index(location.origin) != -1; # NOT OK, but not flagged
46+
x.index(getCurrentDomain()) != nil; # NOT OK, but not flagged
47+
x.index(location.origin) != nil; # NOT OK, but not flagged
4848

4949
x.index("tar.gz") + offset; # OK
5050
x.index("tar.gz") - offset; # OK
5151

52-
x.index("https://example.internal") != -1; # NOT OK
53-
x.index("https://") != -1; # OK
52+
x.index("https://example.internal") != nil; # NOT OK
53+
x.index("https://") != nil; # OK
5454

5555
x.start_with?("https://example.internal"); # NOT OK
5656
x.index('https://example.internal.org') != 0; # NOT OK
5757
x.index('https://example.internal.org') === 0; # NOT OK
5858
x.end_with?("internal.com"); # NOT OK
5959
x.start_with?("https://example.internal:80"); # OK
6060

61-
x.index("secure.com") != -1; # NOT OK
62-
x.index("secure.com") === -1; # OK
63-
!(x.index("secure.com") != -1); # OK
61+
x.index("secure.com") != nil; # NOT OK
62+
x.index("secure.com") === nil; # OK
63+
!(x.index("secure.com") != nil); # OK
6464
!x.include?("secure.com"); # OK
6565

6666
if !x.include?("secure.com") # NOT OK

0 commit comments

Comments
 (0)