Merge pull request github#14209 from geoffw0/regexport

Swift: Port regex mode flag fix from Python to Swift

Author: geoffw0

swift/ql/lib/change-notes/2023-09-12-regex-mode-flag-groups.md

@@ -0,0 +1,4 @@
+---
+category: fix
+---
+* The regular expressions library no longer incorrectly matches mode flag characters against the input.

swift/ql/lib/change-notes/2023-09-26-regex-mode-flags.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The regular expressions library now accepts a wider range of mode flags in a regular expression mode flag group (such as `(?u)`). The `(?w`) flag has been renamed from "UNICODE" to "UNICODEBOUNDARY", and the `(?u)` flag is called "UNICODE" in the libraries.

swift/ql/lib/codeql/swift/regex/Regex.qll

@@ -120,7 +120,8 @@ private newtype TRegexParseMode =
   MkVerbose() or // ignores whitespace and `#` comments within patterns
   MkDotAll() or // dot matches all characters, including line terminators
   MkMultiLine() or // `^` and `$` also match beginning and end of lines
-  MkUnicode() // Unicode UAX 29 word boundary mode
+  MkUnicodeBoundary() or // Unicode UAX 29 word boundary mode
+  MkUnicode() // Unicode matching
 
 /**
  * A regular expression parse mode flag.
@@ -138,6 +139,8 @@ class RegexParseMode extends TRegexParseMode {
     or
     this = MkMultiLine() and result = "MULTILINE"
     or
+    this = MkUnicodeBoundary() and result = "UNICODEBOUNDARY"
+    or
     this = MkUnicode() and result = "UNICODE"
   }
 
@@ -249,7 +252,7 @@ class NSRegularExpressionRegexAdditionalFlowStep extends RegexAdditionalFlowStep
         .getMember()
         .(FieldDecl)
         .hasQualifiedName("NSRegularExpression.Options", "useUnicodeWordBoundaries") and
-    mode = MkUnicode() and
+    mode = MkUnicodeBoundary() and
     isSet = true
   }
 }

swift/ql/lib/codeql/swift/regex/internal/ParseRegex.qll

@@ -1,6 +1,9 @@
 /**
  * Library for parsing Swift regular expressions.
  *
+ * See https://developer.apple.com/documentation/foundation/nsregularexpression
+ * for the regular expression syntax we aim to support.
+ *
  * N.B. does not yet handle stripping whitespace and comments in regexes with
  * the `x` (free-spacing) flag.
  */
@@ -9,6 +12,17 @@ import swift
 private import RegexTracking
 private import codeql.swift.regex.Regex
 
+/**
+ * A mode character that can be used in a regular expression.
+ * ```
+ * NSRegularExpression accepts: dim suwxDPSUW
+ * Regex accepts:                imns  x
+ * ```
+ */
+private predicate availableRegexModeCharacter(string char) {
+  char = ["d", "i", "m", "n", "s", "u", "w", "x", "D", "P", "S", "U", "W"]
+}
+
 /**
  * A `Expr` containing a regular expression term, that is, either
  * a regular expression literal, or a string literal used in a context where
@@ -277,35 +291,54 @@ abstract class RegExp extends Expr {
   private predicate isGroupStart(int i) { this.nonEscapedCharAt(i) = "(" and not this.inCharSet(i) }
 
   /**
-   * Holds if a parse mode starts between `start` and `end`.
+   * Holds if the initial part of a parse mode, not containing any
+   * mode characters is between `start` and `end`.
    */
-  private predicate flagGroupStart(int start, int end) {
+  private predicate flagGroupStartNoModes(int start, int end) {
     this.isGroupStart(start) and
     this.getChar(start + 1) = "?" and
-    this.getChar(start + 2) in ["i", "x", "s", "m", "w"] and
+    availableRegexModeCharacter(this.getChar(start + 2)) and
     end = start + 2
   }
 
   /**
-   * Holds if a parse mode group is between `start` and `end`, and includes the
-   * mode flag `c`. For example the following span, with mode flag `i`:
+   * Holds if `pos` contains a mode character from the
+   * flag group starting at `start`.
+   */
+  private predicate modeCharacter(int start, int pos) {
+    this.flagGroupStartNoModes(start, pos)
+    or
+    this.modeCharacter(start, pos - 1) and
+    availableRegexModeCharacter(this.getChar(pos))
+  }
+
+  /**
+   * Holds if a parse mode group is between `start` and `end`.
+   */
+  private predicate flagGroupStart(int start, int end) {
+    this.flagGroupStartNoModes(start, _) and
+    end = max(int i | this.modeCharacter(start, i) | i + 1)
+  }
+
+  /**
+   * Holds if a parse mode group of this regex includes the mode flag `c`.
+   * For example the following parse mode group, with mode flag `"i"`:
    * ```
    * (?i)
    * ```
    */
-  private predicate flagGroup(int start, int end, string c) {
-    exists(int inStart, int inEnd |
-      this.flagGroupStart(start, inStart) and
-      this.groupContents(start, end, inStart, inEnd) and
-      this.getChar([inStart .. inEnd - 1]) = c
+  private predicate flag(string c) {
+    exists(int pos |
+      this.modeCharacter(_, pos) and
+      this.getChar(pos) = c
     )
   }
 
   /**
    * Gets a mode of this regular expression string if it is defined by a mode prefix.
    */
   string getModeFromPrefix() {
-    exists(string c | this.flagGroup(_, _, c) |
+    exists(string c | this.flag(c) |
       c = "i" and result = "IGNORECASE" // case insensitive
       or
       c = "x" and result = "VERBOSE" // ignores whitespace and `#` comments within patterns
@@ -314,7 +347,10 @@ abstract class RegExp extends Expr {
       or
       c = "m" and result = "MULTILINE" // `^` and `$` also match beginning and end of lines
       or
-      c = "w" and result = "UNICODE" // Unicode UAX 29 word boundary mode
+      c = "w" and result = "UNICODEBOUNDARY" // Unicode UAX 29 word boundary mode
+      or
+      c = "u" and result = "UNICODE" // Unicode matching
+      // (other flags exist that are not translated here)
     )
   }
 
@@ -325,6 +361,7 @@ abstract class RegExp extends Expr {
    * VERBOSE
    * DOTALL
    * MULTILINE
+   * UNICODEBOUNDARY
    * UNICODE
    */
   string getAMode() {

swift/ql/test/library-tests/regex/parse.expected

@@ -1618,16 +1618,13 @@ redos_variants.swift:
 
 #  142| [RegExpConstant, RegExpNormalChar] !
 
-#  146| [RegExpGroup] (?s)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] s
+#  146| [RegExpZeroWidthMatch] (?s)
 
 #  146| [RegExpSequence] (?s)(.|\n)*!
-#-----| 0 -> [RegExpGroup] (?s)
+#-----| 0 -> [RegExpZeroWidthMatch] (?s)
 #-----| 1 -> [RegExpStar] (.|\n)*
 #-----| 2 -> [RegExpConstant, RegExpNormalChar] !
 
-#  146| [RegExpConstant, RegExpNormalChar] s
-
 #  146| [RegExpGroup] (.|\n)
 #-----| 0 -> [RegExpAlt] .|\n
 
@@ -6492,61 +6489,49 @@ regex.swift:
 
 #  206| [RegExpNamedCharacterProperty] [:aaaaa:]
 
-#  211| [RegExpGroup] (?i)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] i
+#  211| [RegExpZeroWidthMatch] (?i)
 
 #  211| [RegExpSequence] (?i)abc
-#-----| 0 -> [RegExpGroup] (?i)
+#-----| 0 -> [RegExpZeroWidthMatch] (?i)
 #-----| 1 -> [RegExpConstant, RegExpNormalChar] abc
 
-#  211| [RegExpConstant, RegExpNormalChar] i
-
 #  211| [RegExpConstant, RegExpNormalChar] abc
 
-#  212| [RegExpGroup] (?s)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] s
+#  212| [RegExpZeroWidthMatch] (?s)
 
 #  212| [RegExpSequence] (?s)abc
-#-----| 0 -> [RegExpGroup] (?s)
+#-----| 0 -> [RegExpZeroWidthMatch] (?s)
 #-----| 1 -> [RegExpConstant, RegExpNormalChar] abc
 
-#  212| [RegExpConstant, RegExpNormalChar] s
-
 #  212| [RegExpConstant, RegExpNormalChar] abc
 
-#  213| [RegExpGroup] (?is)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] is
+#  213| [RegExpZeroWidthMatch] (?is)
 
 #  213| [RegExpSequence] (?is)abc
-#-----| 0 -> [RegExpGroup] (?is)
+#-----| 0 -> [RegExpZeroWidthMatch] (?is)
 #-----| 1 -> [RegExpConstant, RegExpNormalChar] abc
 
-#  213| [RegExpConstant, RegExpNormalChar] is
-
 #  213| [RegExpConstant, RegExpNormalChar] abc
 
 #  214| [RegExpGroup] (?i-s)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] i-s
+#-----| 0 -> [RegExpConstant, RegExpNormalChar] -s
 
 #  214| [RegExpSequence] (?i-s)abc
 #-----| 0 -> [RegExpGroup] (?i-s)
 #-----| 1 -> [RegExpConstant, RegExpNormalChar] abc
 
-#  214| [RegExpConstant, RegExpNormalChar] i-s
+#  214| [RegExpConstant, RegExpNormalChar] -s
 
 #  214| [RegExpConstant, RegExpNormalChar] abc
 
 #  217| [RegExpConstant, RegExpNormalChar] abc
 
 #  217| [RegExpSequence] abc(?i)def
 #-----| 0 -> [RegExpConstant, RegExpNormalChar] abc
-#-----| 1 -> [RegExpGroup] (?i)
+#-----| 1 -> [RegExpZeroWidthMatch] (?i)
 #-----| 2 -> [RegExpConstant, RegExpNormalChar] def
 
-#  217| [RegExpGroup] (?i)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] i
-
-#  217| [RegExpConstant, RegExpNormalChar] i
+#  217| [RegExpZeroWidthMatch] (?i)
 
 #  217| [RegExpConstant, RegExpNormalChar] def
 
@@ -6558,16 +6543,13 @@ regex.swift:
 #-----| 2 -> [RegExpConstant, RegExpNormalChar] ghi
 
 #  218| [RegExpGroup] (?i:def)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] i:def
+#-----| 0 -> [RegExpConstant, RegExpNormalChar] :def
 
-#  218| [RegExpConstant, RegExpNormalChar] i:def
+#  218| [RegExpConstant, RegExpNormalChar] :def
 
 #  218| [RegExpConstant, RegExpNormalChar] ghi
 
-#  219| [RegExpGroup] (?i)
-#-----| 0 -> [RegExpConstant, RegExpNormalChar] i
-
-#  219| [RegExpConstant, RegExpNormalChar] i
+#  219| [RegExpZeroWidthMatch] (?i)
 
 #  219| [RegExpConstant, RegExpNormalChar] abc
 

swift/ql/test/library-tests/regex/regex.swift

@@ -211,7 +211,7 @@ func myRegexpMethodsTests(b: Bool, str_unknown: String) throws {
     _ = try Regex("(?i)abc").firstMatch(in: input) // $ input=input modes=IGNORECASE regex=(?i)abc
     _ = try Regex("(?s)abc").firstMatch(in: input) // $ input=input modes=DOTALL regex=(?s)abc
     _ = try Regex("(?is)abc").firstMatch(in: input) // $ input=input modes="DOTALL | IGNORECASE" regex=(?is)abc
-    _ = try Regex("(?i-s)abc").firstMatch(in: input) // $ input=input regex=(?i-s)abc MISSING: modes=IGNORECASE SPURIOUS: modes="DOTALL | IGNORECASE"
+    _ = try Regex("(?i-s)abc").firstMatch(in: input) // $ input=input regex=(?i-s)abc modes=IGNORECASE
 
     // these cases use parse modes on localized areas of the regex, which we don't currently support
     _ = try Regex("abc(?i)def").firstMatch(in: input) // $ input=input modes=IGNORECASE regex=abc(?i)def

swift/ql/test/query-tests/Security/CWE-116/BadTagFilter.expected

@@ -1,18 +1,24 @@
 | test.swift:79:26:79:48 | <script.*?>.*?<\\/script> | This regular expression does not match script end tags like </script >. |
+| test.swift:83:27:83:54 | (?is)<script.*?>.*?<\\/script> | This regular expression does not match script end tags like </script >. |
 | test.swift:86:27:86:49 | <script.*?>.*?<\\/script> | This regular expression does not match script end tags like </script >. |
 | test.swift:90:50:90:72 | <script.*?>.*?<\\/script> | This regular expression does not match script end tags like </script >. |
 | test.swift:113:26:113:35 | <!--.*--!?> | This regular expression does not match comments containing newlines. |
 | test.swift:117:26:117:58 | <script.*?>(.\|\\s)*?<\\/script[^>]*> | This regular expression matches <script></script>, but not <script \\n></script> |
 | test.swift:121:26:121:56 | <script[^>]*?>.*?<\\/script[^>]*> | This regular expression matches <script>...</script>, but not <script >...\\n</script> |
 | test.swift:125:26:125:63 | <script(\\s\|\\w\|=\|")*?>.*?<\\/script[^>]*> | This regular expression does not match script tags where the attribute uses single-quotes. |
+| test.swift:129:28:129:70 | (?is)<script(\\s\|\\w\|=\|')*?>.*?<\\/script[^>]*> | This regular expression does not match script tags where the attribute uses double-quotes. |
 | test.swift:132:28:132:65 | <script(\\s\|\\w\|=\|')*?>.*?<\\/script[^>]*> | This regular expression does not match script tags where the attribute uses double-quotes. |
 | test.swift:136:50:136:87 | <script(\\s\|\\w\|=\|')*?>.*?<\\/script[^>]*> | This regular expression does not match script tags where the attribute uses double-quotes. |
+| test.swift:140:28:140:74 | (?is)<script( \|\\n\|\\w\|=\|'\|")*?>.*?<\\/script[^>]*> | This regular expression does not match script tags where tabs are used between attributes. |
 | test.swift:143:28:143:69 | <script( \|\\n\|\\w\|=\|'\|")*?>.*?<\\/script[^>]*> | This regular expression does not match script tags where tabs are used between attributes. |
 | test.swift:147:50:147:91 | <script( \|\\n\|\\w\|=\|'\|")*?>.*?<\\/script[^>]*> | This regular expression does not match script tags where tabs are used between attributes. |
+| test.swift:151:28:151:59 | (?s)<script.*?>.*?<\\/script[^>]*> | This regular expression does not match upper case <SCRIPT> tags. |
 | test.swift:154:28:154:55 | <script.*?>.*?<\\/script[^>]*> | This regular expression does not match upper case <SCRIPT> tags. |
 | test.swift:157:50:157:77 | <script.*?>.*?<\\/script[^>]*> | This regular expression does not match upper case <SCRIPT> tags. |
+| test.swift:161:28:161:77 | (?s)<(script\|SCRIPT).*?>.*?<\\/(script\|SCRIPT)[^>]*> | This regular expression does not match mixed case <sCrIpT> tags. |
 | test.swift:164:28:164:73 | <(script\|SCRIPT).*?>.*?<\\/(script\|SCRIPT)[^>]*> | This regular expression does not match mixed case <sCrIpT> tags. |
 | test.swift:167:50:167:95 | <(script\|SCRIPT).*?>.*?<\\/(script\|SCRIPT)[^>]*> | This regular expression does not match mixed case <sCrIpT> tags. |
+| test.swift:171:28:171:64 | (?i)<script[^>]*?>[\\s\\S]*?<\\/script.*> | This regular expression does not match script end tags like </script\\t\\n bar>. |
 | test.swift:174:28:174:60 | <script[^>]*?>[\\s\\S]*?<\\/script.*> | This regular expression does not match script end tags like </script\\t\\n bar>. |
 | test.swift:177:50:177:82 | <script[^>]*?>[\\s\\S]*?<\\/script.*> | This regular expression does not match script end tags like </script\\t\\n bar>. |
 | test.swift:191:27:191:68 | <(?:!--([\\S\|\\s]*?)-->)\|([^\\/\\s>]+)[\\S\\s]*?> | Comments ending with --> are matched differently from comments ending with --!>. The first is matched with capture group 1 and comments ending with --!> are matched with capture group 2. |

swift/ql/test/query-tests/Security/CWE-116/test.swift

@@ -79,7 +79,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
     let re1 = try Regex(#"<script.*?>.*?<\/script>"#).ignoresCase(true)
     _ = try re1.firstMatch(in: tainted)
 
-    // BAD - doesn't match `</script >` [NOT DETECTED - all regexs with mode flags are currently missed by the query]
+    // BAD - doesn't match `</script >`
     let re2a = try Regex(#"(?is)<script.*?>.*?<\/script>"#)
     _ = try re2a.firstMatch(in: tainted)
     // BAD - doesn't match `</script >`
@@ -125,7 +125,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
     let re9 = try Regex(#"<script(\s|\w|=|")*?>.*?<\/script[^>]*>"#).ignoresCase(true).dotMatchesNewlines(true)
     _ = try re9.firstMatch(in: tainted)
 
-    // BAD - does not match double quotes for attribute values [NOT DETECTED]
+    // BAD - does not match double quotes for attribute values
     let re10a = try Regex(#"(?is)<script(\s|\w|=|')*?>.*?<\/script[^>]*>"#)
     _ = try re10a.firstMatch(in: tainted)
     // BAD - does not match double quotes for attribute values
@@ -136,7 +136,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
     let ns10 = try NSRegularExpression(pattern: #"<script(\s|\w|=|')*?>.*?<\/script[^>]*>"#, options: options10)
     _ = ns10.firstMatch(in: tainted, range: NSMakeRange(0, tainted.utf16.count))
 
-    // BAD - does not match tabs between attributes [NOT DETECTED]
+    // BAD - does not match tabs between attributes
     let re11a = try Regex(#"(?is)<script( |\n|\w|=|'|")*?>.*?<\/script[^>]*>"#)
     _ = try re11a.firstMatch(in: tainted)
     // BAD - does not match tabs between attributes
@@ -147,7 +147,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
     let ns11 = try NSRegularExpression(pattern: #"<script( |\n|\w|=|'|")*?>.*?<\/script[^>]*>"#, options: options11)
     _ = ns11.firstMatch(in: tainted, range: NSMakeRange(0, tainted.utf16.count))
 
-    // BAD - does not match uppercase SCRIPT tags [NOT DETECTED]
+    // BAD - does not match uppercase SCRIPT tags
     let re12a = try Regex(#"(?s)<script.*?>.*?<\/script[^>]*>"#)
     _ = try re12a.firstMatch(in: tainted)
     // BAD - does not match uppercase SCRIPT tags
@@ -157,7 +157,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
     let ns12 = try NSRegularExpression(pattern: #"<script.*?>.*?<\/script[^>]*>"#, options: .dotMatchesLineSeparators)
     _ = ns12.firstMatch(in: tainted, range: NSMakeRange(0, tainted.utf16.count))
 
-    // BAD - does not match mixed case script tags [NOT DETECTED]
+    // BAD - does not match mixed case script tags
     let re13a = try Regex(#"(?s)<(script|SCRIPT).*?>.*?<\/(script|SCRIPT)[^>]*>"#)
     _ = try re13a.firstMatch(in: tainted)
     // BAD - does not match mixed case script tags
@@ -167,7 +167,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
     let ns13 = try NSRegularExpression(pattern: #"<(script|SCRIPT).*?>.*?<\/(script|SCRIPT)[^>]*>"#, options: .dotMatchesLineSeparators)
     _ = ns13.firstMatch(in: tainted, range: NSMakeRange(0, tainted.utf16.count))
 
-    // BAD - doesn't match newlines in the end tag [NOT DETECTED]
+    // BAD - doesn't match newlines in the end tag
     let re14a = try Regex(#"(?i)<script[^>]*?>[\s\S]*?<\/script.*>"#)
     _ = try re14a.firstMatch(in: tainted)
     // BAD - doesn't match newlines in the end tag

swift/ql/test/query-tests/Security/CWE-1333/ReDoS.expected

@@ -2,6 +2,7 @@
 | ReDoS.swift:65:22:65:22 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
 | ReDoS.swift:66:22:66:22 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
 | ReDoS.swift:69:18:69:18 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
-| ReDoS.swift:75:46:75:46 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
-| ReDoS.swift:77:57:77:57 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
-| ReDoS.swift:80:57:80:57 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
+| ReDoS.swift:73:26:73:33 | (?:.\|\\n)* | This part of the regular expression may cause exponential backtracking on strings starting with 'x' and containing many repetitions of '\\n'. |
+| ReDoS.swift:77:46:77:46 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
+| ReDoS.swift:79:57:79:57 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |
+| ReDoS.swift:82:57:82:57 | a* | This part of the regular expression may cause exponential backtracking on strings containing many repetitions of 'a'. |

swift/ql/test/query-tests/Security/CWE-1333/ReDoS.swift

@@ -70,6 +70,8 @@ func myRegexpTests(myUrl: URL) throws {
     let regex = try Regex(str)
     _ = try regex.firstMatch(in: tainted)
 
+    _ = try Regex(#"(?is)X(?:.|\n)*Y"#) // BAD - suggested attack should begin with 'x' or 'X', *not* 'isx' or 'isX'
+
     // NSRegularExpression
 
     _ = try? NSRegularExpression(pattern: "((a*)*b)") // DUBIOUS (never used) [FLAGGED]