JS: add RegExpPatternSource::getAParse to hide the subclasses

Esben Sparre Andreasen · Esben Sparre Andreasen · commit bf51c54338c2 · 2019-06-03T14:23:22.000+02:00
diff --git a/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql b/javascript/ql/src/Security/CWE-020/IncompleteHostnameRegExp.ql
@@ -31,10 +31,10 @@ where
   pattern = re.getPattern() and
   isIncompleteHostNameRegExpPattern(pattern, hostPart) and
   (
-    if re instanceof StringRegExpPatternSource
+    if re.getAParse() != re
     then (
       kind = "string, which is used as a regular expression $@," and
-      aux = re.(StringRegExpPatternSource).getAUse()
+      aux = re.getAParse()
     ) else (
       kind = "regular expression" and aux = re
     )
diff --git a/javascript/ql/src/Security/CWE-020/MissingRegExpAnchor.ql b/javascript/ql/src/Security/CWE-020/MissingRegExpAnchor.ql
@@ -63,7 +63,7 @@ predicate isInterestingUnanchoredRegExpString(RegExpPatternSource src, string ms
         arg = mcn.getArgument(0) and
         (
           src.getARegExpObject().flowsTo(arg) or
-          src.(StringRegExpPatternSource).getAUse() = arg
+          src.getAParse() = arg
         )
       |
         name = "replace"
diff --git a/javascript/ql/src/semmle/javascript/Regexp.qll b/javascript/ql/src/semmle/javascript/Regexp.qll
@@ -438,6 +438,12 @@ private DataFlow::Node regExpSource(DataFlow::Node re) {
  * as a part of a regular expression.
  */
 abstract class RegExpPatternSource extends DataFlow::Node {
+  /**
+   * Gets a node where the pattern of this node is parsed as a part of
+   * a regular expression.
+   */
+  abstract DataFlow::Node getAParse();
+
   /**
    * Gets the pattern of this node that is interpreted as a part of a
    * regular expression.
@@ -454,7 +460,7 @@ abstract class RegExpPatternSource extends DataFlow::Node {
 /**
  * A regular expression literal, viewed as the pattern source for itself.
  */
-class RegExpLiteralPatternSource extends RegExpPatternSource {
+private class RegExpLiteralPatternSource extends RegExpPatternSource {
   string pattern;
 
   RegExpLiteralPatternSource() {
@@ -464,6 +470,8 @@ class RegExpLiteralPatternSource extends RegExpPatternSource {
     )
   }
 
+  override DataFlow::Node getAParse() { result = this }
+
   override string getPattern() { result = pattern }
 
   override DataFlow::SourceNode getARegExpObject() { result = this }
@@ -473,20 +481,17 @@ class RegExpLiteralPatternSource extends RegExpPatternSource {
  * A node whose string value may flow to a position where it is interpreted
  * as a part of a regular expression.
  */
-class StringRegExpPatternSource extends RegExpPatternSource {
-  DataFlow::Node use;
+private class StringRegExpPatternSource extends RegExpPatternSource {
+  DataFlow::Node parse;
 
-  StringRegExpPatternSource() { this = regExpSource(use) }
+  StringRegExpPatternSource() { this = regExpSource(parse) }
 
-  /**
-   * Gets a node that uses this source as a regular expression pattern.
-   */
-  DataFlow::Node getAUse() { result = use }
+  override DataFlow::Node getAParse() { result = parse }
 
   override DataFlow::SourceNode getARegExpObject() {
     exists(DataFlow::InvokeNode constructor |
       constructor = DataFlow::globalVarRef("RegExp").getAnInvocation() and
-      use = constructor.getArgument(0) and
+      parse = constructor.getArgument(0) and
       result = constructor
     )
   }
diff --git a/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected b/javascript/ql/test/query-tests/Security/CWE-020/IncompleteHostnameRegExp.expected
@@ -1,9 +1,9 @@
 | tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:3:2:3:29 | /^http: ... le.com/ | here |
 | tst-IncompleteHostnameRegExp.js:5:2:5:29 | /^http: ... le.net/ | This regular expression has an unescaped '.' before 'example.net', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:5:2:5:29 | /^http: ... le.net/ | here |
 | tst-IncompleteHostnameRegExp.js:6:2:6:43 | /^http: ... b).com/ | This regular expression has an unescaped '.' before '(example-a\|example-b).com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:6:2:6:43 | /^http: ... b).com/ | here |
-| tst-IncompleteHostnameRegExp.js:11:13:11:38 | "^http: ... le.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.js:11:13:11:38 | "^http: ... le.com" | here |
-| tst-IncompleteHostnameRegExp.js:12:10:12:35 | "^http: ... le.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.js:12:10:12:35 | "^http: ... le.com" | here |
-| tst-IncompleteHostnameRegExp.js:17:13:17:31 | `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.js:17:13:17:31 | `test.example.com$` | here |
+| tst-IncompleteHostnameRegExp.js:11:13:11:38 | "^http: ... le.com" | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:11:13:11:38 | "^http: ... le.com" | here |
+| tst-IncompleteHostnameRegExp.js:12:10:12:35 | "^http: ... le.com" | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:12:10:12:35 | "^http: ... le.com" | here |
+| tst-IncompleteHostnameRegExp.js:17:13:17:31 | `test.example.com$` | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:17:13:17:31 | `test.example.com$` | here |
 | tst-IncompleteHostnameRegExp.js:17:14:17: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.js:17:13:17:31 | `test.example.com$` | here |
 | tst-IncompleteHostnameRegExp.js:19:17:19:35 | '^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.js:20:13:20:26 | `${hostname}$` | here |
 | tst-IncompleteHostnameRegExp.js:22:27:22:45 | '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.js:23:13:23:27 | domain.hostname | here |
@@ -13,15 +13,15 @@
 | tst-IncompleteHostnameRegExp.js:38:2:38:44 | /^(http ... p\\/f\\// | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:38:2:38:44 | /^(http ... p\\/f\\// | here |
 | tst-IncompleteHostnameRegExp.js:39:2:39:33 | /^(http ... om\\/)/g | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:39:2:39:33 | /^(http ... om\\/)/g | here |
 | tst-IncompleteHostnameRegExp.js:40:2:40:30 | /^https ... le.com/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:40:2:40:30 | /^https ... le.com/ | here |
-| tst-IncompleteHostnameRegExp.js:41:13:41:68 | '^http: ... e\\.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.js:41:13:41:68 | '^http: ... e\\.com' | here |
+| tst-IncompleteHostnameRegExp.js:41:13:41:68 | '^http: ... e\\.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:41:13:41:68 | '^http: ... e\\.com' | here |
 | tst-IncompleteHostnameRegExp.js:41:41:41:68 | '^https ... e\\.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.js:41:13:41:68 | '^http: ... e\\.com' | here |
-| tst-IncompleteHostnameRegExp.js:42:13:42:62 | '^http[ ... \\/(.+)' | 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.js:42:13:42:62 | '^http[ ... \\/(.+)' | here |
+| tst-IncompleteHostnameRegExp.js:42:13:42:62 | '^http[ ... \\/(.+)' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:42:13:42:62 | '^http[ ... \\/(.+)' | here |
 | tst-IncompleteHostnameRegExp.js:43:2:43:33 | /^https ... e.com$/ | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:43:2:43:33 | /^https ... e.com$/ | here |
-| tst-IncompleteHostnameRegExp.js:44:9:44:101 | '^proto ... ernal)' | This string, which is used as a regular expression $@, has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:44:9:44:101 | '^proto ... ernal)' | here |
+| tst-IncompleteHostnameRegExp.js:44:9:44:101 | '^proto ... ernal)' | This regular expression has an unescaped '.' before 'example-b.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:44:9:44:101 | '^proto ... ernal)' | here |
 | tst-IncompleteHostnameRegExp.js:46:2:46:29 | /^(exam ... e.com)/ | This regular expression has an unescaped '.' before 'dev\|example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:46:2:46:29 | /^(exam ... e.com)/ | here |
-| tst-IncompleteHostnameRegExp.js:48:13:48:68 | '^http: ... e\\.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.js:48:13:48:68 | '^http: ... e\\.com' | here |
+| tst-IncompleteHostnameRegExp.js:48:13:48:68 | '^http: ... e\\.com' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:48:13:48:68 | '^http: ... e\\.com' | here |
 | tst-IncompleteHostnameRegExp.js:48:41:48:68 | '^https ... e\\.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.js:48:13:48:68 | '^http: ... e\\.com' | here |
-| tst-IncompleteHostnameRegExp.js:53:13:53:36 | 'test.' ... e.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.js:53:13:53:36 | 'test.' ... e.com$' | here |
+| tst-IncompleteHostnameRegExp.js:53:13:53:36 | 'test.' ... e.com$' | This regular expression has an unescaped '.' before 'example.com', so it might match more hosts than expected. | tst-IncompleteHostnameRegExp.js:53:13:53:36 | 'test.' ... e.com$' | here |
 | tst-SemiAnchoredRegExp.js:30:2:30:23 | /^good. ... er.com/ | This regular expression has an unescaped '.' before 'com\|better.com', so it might match more hosts than expected. | tst-SemiAnchoredRegExp.js:30:2:30:23 | /^good. ... er.com/ | here |
-| tst-SemiAnchoredRegExp.js:66:13:66:34 | '^good. ... er.com' | This string, which is used as a regular expression $@, has an unescaped '.' before 'com\|better.com', so it might match more hosts than expected. | tst-SemiAnchoredRegExp.js:66:13:66:34 | '^good. ... er.com' | here |
-| tst-SemiAnchoredRegExp.js:67:13:67:36 | '^good\\ ... r\\.com' | This string, which is used as a regular expression $@, has an unescaped '.' before 'com\|better.com', so it might match more hosts than expected. | tst-SemiAnchoredRegExp.js:67:13:67:36 | '^good\\ ... r\\.com' | here |
+| tst-SemiAnchoredRegExp.js:66:13:66:34 | '^good. ... er.com' | This regular expression has an unescaped '.' before 'com\|better.com', so it might match more hosts than expected. | tst-SemiAnchoredRegExp.js:66:13:66:34 | '^good. ... er.com' | here |
+| tst-SemiAnchoredRegExp.js:67:13:67:36 | '^good\\ ... r\\.com' | This regular expression has an unescaped '.' before 'com\|better.com', so it might match more hosts than expected. | tst-SemiAnchoredRegExp.js:67:13:67:36 | '^good\\ ... r\\.com' | here |

Original file line number	Diff line number	Diff line change
`@@ -31,10 +31,10 @@ where`
`31`	`31`	`pattern = re.getPattern() and`
`32`	`32`	`isIncompleteHostNameRegExpPattern(pattern, hostPart) and`
`33`	`33`	`(`
`34`		`- if re instanceof StringRegExpPatternSource`
	`34`	`+ if re.getAParse() != re`
`35`	`35`	`then (`
`36`	`36`	`kind = "string, which is used as a regular expression $@," and`
`37`		`- aux = re.(StringRegExpPatternSource).getAUse()`
	`37`	`+ aux = re.getAParse()`
`38`	`38`	`) else (`
`39`	`39`	`kind = "regular expression" and aux = re`
`40`	`40`	`)`
Original file line number	Diff line number	Diff line change
`@@ -63,7 +63,7 @@ predicate isInterestingUnanchoredRegExpString(RegExpPatternSource src, string ms`
`63`	`63`	`arg = mcn.getArgument(0) and`
`64`	`64`	`(`
`65`	`65`	`src.getARegExpObject().flowsTo(arg) or`
`66`		`- src.(StringRegExpPatternSource).getAUse() = arg`
	`66`	`+ src.getAParse() = arg`
`67`	`67`	`)`
`68`	`68`	`\|`
`69`	`69`	`name = "replace"`