Add SPURIOUS and MISSING to some comments

Author: owen-mc

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

@@ -27,7 +27,7 @@ def foo
 
 	convert1({ hostname: 'test.example.com$' }); # $ Alert // NOT OK
 
-	domains = [ { hostname: 'test.example.com$' } ];  # NOT OK - but not flagged due to limitations of TypeTracking.
+	domains = [ { hostname: 'test.example.com$' } ]; # $ MISSING: Alert # NOT OK - but not flagged due to limitations of TypeTracking.
 
 
 
@@ -39,7 +39,7 @@ def foo
 	/^(http:\/\/sub.example.com\/)/i; # $ Alert // NOT OK
 	/^https?:\/\/api.example.com/; # $ Alert // NOT OK
 	Regexp.new('^http://localhost:8000|' + "^https?://.+\\.example\\.com/"); # $ Alert // NOT OK
-	Regexp.new("^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)"); # NOT OK
+	Regexp.new("^http[s]?:\/\/?sub1\\.sub2\\.example\\.com\/f\/(.+)"); # $ MISSING: Alert # NOT OK
 	/^https:\/\/[a-z]*.example.com$/; # $ Alert // NOT OK
 	Regexp.compile('^protos?://(localhost|.+.example.net|.+.example-a.com|.+.example-b.com|.+.example.internal)'); # $ Alert // NOT OK
 
@@ -48,11 +48,11 @@ def foo
 	Regexp.new('^http://localhost:8000|' + "^https?://.+.example\\.com/"); # $ Alert // NOT OK
 
 	primary = 'example.com$';
-	Regexp.new('test.' + primary); # NOT OK, but not detected
+	Regexp.new('test.' + primary); # $ MISSING: Alert # NOT OK, but not detected
 
-	Regexp.new('test.' + 'example.com$'); # NOT OK
+	Regexp.new('test.' + 'example.com$'); # $ MISSING: Alert # NOT OK
 
-	Regexp.new('^http://test\.example.com'); # NOT OK
+	Regexp.new('^http://test\.example.com'); # $ MISSING: Alert # NOT OK
 
 	/^http:\/\/(..|...)\.example\.com\/index\.html/; # OK, wildcards are intentional
 	/^http:\/\/.\.example\.com\/index\.html/; # OK, the wildcard is intentional

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

@@ -1,11 +1,11 @@
 def test (x)
-    x.index("internal") != nil; # NOT OK, but not flagged
-    x.index("localhost") != nil; # NOT OK, but not flagged
+    x.index("internal") != nil; # $ MISSING: Alert # NOT OK, but not flagged
+    x.index("localhost") != nil; # $ MISSING: Alert # NOT OK, but not flagged
     x.index("secure.com") != nil; # $ Alert // NOT OK
     x.index("secure.net") != nil; # $ Alert // NOT OK
     x.index(".secure.com") != nil; # $ Alert // NOT OK
-    x.index("sub.secure.") != nil; # NOT OK, but not flagged
-    x.index(".sub.secure.") != nil; # NOT OK, but not flagged
+    x.index("sub.secure.") != nil; # $ MISSING: Alert # NOT OK, but not flagged
+    x.index(".sub.secure.") != nil; # $ MISSING: Alert # NOT OK, but not flagged
 
     x.index("secure.com") === nil; # $ Alert // NOT OK
     x.index("secure.com") === 0; # $ Alert // NOT OK
@@ -33,7 +33,7 @@ def test (x)
     x.index("https://secure.com:443") != nil; # $ Alert // NOT OK
     x.index("https://secure.com/") != nil; # $ Alert // NOT OK
 
-    x.index(".cn") != nil; # NOT OK, but not flagged
+    x.index(".cn") != nil; # $ MISSING: Alert # NOT OK, but not flagged
     x.index(".jpg") != nil; # OK
     x.index("index.html") != nil; # OK
     x.index("index.js") != nil; # OK
@@ -43,8 +43,8 @@ def test (x)
     x.index("secure=true") != nil; # OK (query param)
     x.index("&auth=") != nil; # OK (query param)
 
-    x.index(getCurrentDomain()) != nil; # NOT OK, but not flagged
-    x.index(location.origin) != nil; # NOT OK, but not flagged
+    x.index(getCurrentDomain()) != nil; # $ MISSING: Alert # NOT OK, but not flagged
+    x.index(location.origin) != nil; # $ MISSING: Alert # NOT OK, but not flagged
 
 	x.index("tar.gz") + offset; # OK
 	x.index("tar.gz") - offset; # OK
@@ -68,7 +68,7 @@ def test (x)
 	else
 		doSomeThingWithTrustedURL(x);
     end
-    
+
     x.start_with?("https://secure.com/foo/bar"); # OK - a forward slash after the domain makes prefix checks safe.
     x.index("https://secure.com/foo/bar") >= 0 # $ Alert // NOT OK - the url can be anywhere in the string.
     x.index("https://secure.com") >= 0 # $ Alert // NOT OK

ruby/ql/test/query-tests/security/cwe-116/IncompleteSanitization/tst.rb

@@ -220,7 +220,7 @@ def good13a(s)
   s = s.sub('[', '') # OK
   s = s.sub(']', '') # OK
   s.sub(/{/, '').sub(/}/, '') # OK
-  s.sub(']', '').sub('[', '') # $ Alert // probably OK, but still flagged
+  s.sub(']', '').sub('[', '') # $ SPURIOUS: Alert // probably OK, but still flagged
 end
 
 def good13b(s1)

ruby/ql/test/query-tests/security/cwe-1333-exponential-redos/tst.rb

@@ -304,10 +304,10 @@
 # NOT GOOD
 bad67 = /(\d(\s+)*){20}/ # $ Alert
 
-# GOOD - but we spuriously conclude that a rejecting suffix exists. 
+# GOOD - but we spuriously conclude that a rejecting suffix exists.
 good36 = /(([^\/]|X)+)(\/[\S\s]*)*$/ # $ Alert
 
-# GOOD - but we spuriously conclude that a rejecting suffix exists. 
+# GOOD - but we spuriously conclude that a rejecting suffix exists.
 good37 = /^((x([^Y]+)?)*(Y|$))/ # $ Alert
 
 # NOT GOOD
@@ -326,18 +326,18 @@
 good38 = /(a?)*b/
 
 # NOT GOOD - but not detected
-bad72 = /(c?a?)*b/
+bad72 = /(c?a?)*b/ # $ MISSING: Alert
 
 # NOT GOOD
 bad73 = /(?:a|a?)+b/ # $ Alert
 
-# NOT GOOD - but not detected. 
-bad74 = /(a?b?)*$/
+# NOT GOOD - but not detected.
+bad74 = /(a?b?)*$/ # $ MISSING: Alert
 
 # NOT GOOD
 bad76 = /PRE(([a-c]|[c-d])T(e?e?e?e?|X))+(cTcT|cTXcTX$)/ # $ Alert
 
-# NOT GOOD - but not detected
+# NOT GOOD
 bad77 = /^((a)+\w)+$/ # $ Alert
 
 # NOT GOOD
@@ -362,7 +362,7 @@
 bad85 = /^((?:a{0,|-)|\w\{\d,)+X$/ # $ Alert
 bad86 = /^((?:a{0,2|-)|\w\{\d,\d)+X$/ # $ Alert
 
-# NOT GOOD 
+# NOT GOOD
 bad87 = /^((?:a{0,2}|-)|\w\{\d,\d\})+X$/
 
 # NOT GOOD

ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.rb

@@ -67,7 +67,7 @@ def route8
     # TODO: false positive; we aren't detecting flow from `:json` to the call argument.
     more_options = { allow_blank: true }
     more_options[:mode] = :json
-    object4 = Oj.load json_data, more_options # $ Alert
+    object4 = Oj.load json_data, more_options # $ SPURIOUS: Alert
   end
 
   # GOOD
@@ -127,7 +127,7 @@ def route16
   # GOOD
   def route17
     yaml_data = params[:key]
-    object = Psych.parse_stream(yaml_data) 
+    object = Psych.parse_stream(yaml_data)
     object = Psych.parse(yaml_data)
     object = Psych.parse_file(yaml_data)
   end

ruby/ql/test/query-tests/security/cwe-807-user-controlled-bypass/ConditionalBypass.rb

@@ -18,7 +18,7 @@ def bad_handler2
   def bad_handler3
     # BAD. Not detected: its the last statement in the method, so it doesn't
     # match the heuristic for an action.
-    login if params[:login]
+    login if params[:login] # $ MISSING: Alert
   end
 
   def bad_handler4