Skip to content

Commit 91b2de2

Browse files
geoffw0geoffw0
committed
Swift: Lots of small fixes / cleanup.
1 parent 8e8a9c8 commit 91b2de2

12 files changed

Lines changed: 37 additions & 36 deletions

File tree

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
*/
44

55
import swift
6-
import codeql.swift.dataflow.DataFlow
7-
import codeql.swift.regex.RegexTreeView // re-export
6+
import codeql.swift.regex.RegexTreeView
7+
private import codeql.swift.dataflow.DataFlow
88
private import internal.ParseRegex
99

1010
/**
@@ -54,7 +54,7 @@ private class ParsedStringRegex extends RegExp, StringLiteralExpr {
5454
}
5555

5656
/**
57-
* A call that evaluates a regular expression. For example:
57+
* A call that evaluates a regular expression. For example, the call to `firstMatch` in:
5858
* ```
5959
* Regex("(a|b).*").firstMatch(in: myString)
6060
* ```
@@ -64,7 +64,7 @@ abstract class RegexEval extends CallExpr {
6464
Expr stringInput;
6565

6666
/**
67-
* Gets the input to this call that is the regular expression.
67+
* Gets the input to this call that is the regular expression being evaluated.
6868
*/
6969
Expr getRegexInput() { result = regexInput }
7070

@@ -76,7 +76,7 @@ abstract class RegexEval extends CallExpr {
7676
/**
7777
* Gets a regular expression value that is evaluated here (if any can be identified).
7878
*/
79-
RegExp getARegex() { exists(ParsedStringRegex regex | regex.getEval() = this and result = regex) }
79+
RegExp getARegex() { result.(ParsedStringRegex).getEval() = this }
8080
}
8181

8282
/**

‎swift/ql/lib/codeql/swift/regex/RegexTreeView.qll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ private module Impl implements RegexTreeViewSig {
661661
*
662662
* Examples:
663663
*
664-
* ```rb
664+
* ```
665665
* /[a-fA-F0-9]/
666666
* /[^abc]/
667667
* ```

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

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
150150
_ = try Regex(#"([\w.]+)*"#).firstMatch(in: tainted)
151151
// BAD
152152
// attack string: "a" x lots + "!"
153-
_ = try Regex(#"([\w.]+)*"#).wholeMatch(in: tainted)
153+
_ = try Regex(#"([\w.]+)*"#).wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
154154

155155
// BAD
156156
// attack string: "b" x lots + "!"
@@ -264,13 +264,13 @@ func myRegexpVariantsTests(myUrl: URL) throws {
264264
_ = try Regex(#"(\d+(X\d+)?)+"#).firstMatch(in: tainted)
265265
// BAD
266266
// attack string: "0" x lots + "!"
267-
_ = try Regex(#"(\d+(X\d+)?)+"#).wholeMatch(in: tainted)
267+
_ = try Regex(#"(\d+(X\d+)?)+"#).wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
268268

269269
// GOOD - there is no witness in the end that could cause the regexp to not match
270270
_ = try Regex("([0-9]+(X[0-9]*)?)*").firstMatch(in: tainted)
271271
// BAD
272272
// attack string: "0" x lots + "!"
273-
_ = try Regex("([0-9]+(X[0-9]*)?)*").wholeMatch(in: tainted)
273+
_ = try Regex("([0-9]+(X[0-9]*)?)*").wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
274274

275275
// GOOD
276276
_ = try Regex("^([^>]+)*(>|$)").firstMatch(in: tainted)
@@ -307,7 +307,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
307307
_ = try Regex("(a+)+aaaaa*a+").firstMatch(in: tainted)
308308
// BAD
309309
// attack string: "a" x lots + "!"
310-
_ = try Regex("(a+)+aaaaa*a+").wholeMatch(in: tainted)
310+
_ = try Regex("(a+)+aaaaa*a+").wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
311311

312312
// BAD
313313
// attack string: "a" x lots + "!"
@@ -317,7 +317,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
317317
_ = try Regex(#"(\n+)+\n\n"#).firstMatch(in: tainted)
318318
// BAD
319319
// attack string: "\n" x lots + "."
320-
_ = try Regex(#"(\n+)+\n\n"#).wholeMatch(in: tainted)
320+
_ = try Regex(#"(\n+)+\n\n"#).wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
321321

322322
// BAD
323323
// attack string: "\n" x lots + "."
@@ -335,7 +335,7 @@ func myRegexpVariantsTests(myUrl: URL) throws {
335335
_ = try Regex("(([^X]b)+)*($|[^X]b)").firstMatch(in: tainted)
336336
// BAD
337337
// attack string: "b" x lots + "!"
338-
_ = try Regex("(([^X]b)+)*($|[^X]b)").wholeMatch(in: tainted)
338+
_ = try Regex("(([^X]b)+)*($|[^X]b)").wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
339339

340340
// BAD
341341
// attack string: "b" x lots + "!"
@@ -345,19 +345,19 @@ func myRegexpVariantsTests(myUrl: URL) throws {
345345
_ = try Regex("((ab)+)*ababab").firstMatch(in: tainted)
346346
// BAD
347347
// attack string: "ab" x lots + "!"
348-
_ = try Regex("((ab)+)*ababab").wholeMatch(in: tainted)
348+
_ = try Regex("((ab)+)*ababab").wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
349349

350350
// GOOD
351351
_ = try Regex("((ab)+)*abab(ab)*(ab)+").firstMatch(in: tainted)
352352
// BAD
353353
// attack string: "ab" x lots + "!"
354-
_ = try Regex("((ab)+)*abab(ab)*(ab)+").wholeMatch(in: tainted)
354+
_ = try Regex("((ab)+)*abab(ab)*(ab)+").wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
355355

356356
// GOOD
357357
_ = try Regex("((ab)+)*").firstMatch(in: tainted)
358358
// BAD
359359
// attack string: "ab" x lots + "!"
360-
_ = try Regex("((ab)+)*").wholeMatch(in: tainted)
360+
_ = try Regex("((ab)+)*").wholeMatch(in: tainted) // $ MISSING: redos-vulnerable=
361361

362362
// BAD
363363
// attack string: "ab" x lots + "!"
@@ -480,17 +480,15 @@ func myRegexpVariantsTests(myUrl: URL) throws {
480480
// GOOD
481481
_ = try Regex("(a|b)+").firstMatch(in: tainted)
482482

483-
// BAD
483+
// GOOD
484484
_ = try Regex(#"(?:[\s;,"'<>(){}|\[\]@=+*]|:(?![/\\]))+"#).firstMatch(in: tainted)
485-
// BAD
486-
// attack string: ???
487-
_ = try Regex(#"(?:[\s;,"'<>(){}|\[\]@=+*]|:(?![/\\]))+"#).wholeMatch(in: tainted)
488-
489-
// TODO: investigate; these were marked `hasParseFailure`
490-
_ = try Regex(#"^((?:a{|-)|\w\{)+X$"#).firstMatch(in: tainted) // $ SPURIOUS: redos-vulnerable=
491-
_ = try Regex(#"^((?:a{0|-)|\w\{\d)+X$"#).firstMatch(in: tainted) // $ SPURIOUS: redos-vulnerable=
492-
_ = try Regex(#"^((?:a{0,|-)|\w\{\d,)+X$"#).firstMatch(in: tainted) // $ SPURIOUS: redos-vulnerable=
493-
_ = try Regex(#"^((?:a{0,2|-)|\w\{\d,\d)+X$"#).firstMatch(in: tainted) // $ SPURIOUS: redos-vulnerable=
485+
486+
// BAD?
487+
// (no confirmed attack string)
488+
_ = try Regex(#"^((?:a{|-)|\w\{)+X$"#).firstMatch(in: tainted) // $ redos-vulnerable=
489+
_ = try Regex(#"^((?:a{0|-)|\w\{\d)+X$"#).firstMatch(in: tainted) // $ redos-vulnerable=
490+
_ = try Regex(#"^((?:a{0,|-)|\w\{\d,)+X$"#).firstMatch(in: tainted) // $ redos-vulnerable=
491+
_ = try Regex(#"^((?:a{0,2|-)|\w\{\d,\d)+X$"#).firstMatch(in: tainted) // $ redos-vulnerable=
494492

495493
// GOOD
496494
_ = try Regex(#"^((?:a{0,2}|-)|\w\{\d,\d\})+X$"#).firstMatch(in: tainted)

swift/ql/test/library-tests/regex/licenses/ANodeBlog-LICENSE renamed to swift/ql/test/library-tests/regex/test_fragment_licenses/ANodeBlog-LICENSE

File renamed without changes.

swift/ql/test/library-tests/regex/licenses/CodeMirror-LICENSE renamed to swift/ql/test/library-tests/regex/test_fragment_licenses/CodeMirror-LICENSE

File renamed without changes.

swift/ql/test/library-tests/regex/licenses/Prism-LICENSE renamed to swift/ql/test/library-tests/regex/test_fragment_licenses/Prism-LICENSE

File renamed without changes.

swift/ql/test/library-tests/regex/licenses/Prototype.js-LICENSE renamed to swift/ql/test/library-tests/regex/test_fragment_licenses/Prototype.js-LICENSE

File renamed without changes.

swift/ql/test/library-tests/regex/licenses/brace-expansion-LICENSE renamed to swift/ql/test/library-tests/regex/test_fragment_licenses/brace-expansion-LICENSE

File renamed without changes.

swift/ql/test/library-tests/regex/licenses/jest-LICENSE renamed to swift/ql/test/library-tests/regex/test_fragment_licenses/jest-LICENSE

File renamed without changes.

swift/ql/test/library-tests/regex/licenses/knockout-LICENSE renamed to swift/ql/test/library-tests/regex/test_fragment_licenses/knockout-LICENSE

File renamed without changes.

0 commit comments

Comments
 (0)