Skip to content

Commit 3179c60

Browse files
committed
Ruby: Remove RegExpLiteral.getAMatch
This predicate is a duplicate of getAMatchedString, which matches the naming in the JS version.
1 parent 6bb24f9 commit 3179c60

File tree

2 files changed

+5
-28
lines changed

2 files changed

+5
-28
lines changed

ruby/ql/lib/codeql/ruby/regexp/RegExpTreeView.qll

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -271,9 +271,6 @@ class RegExpTerm extends RegExpParent {
271271

272272
/** Holds if this regular expression term can match the empty string. */
273273
predicate isNullable() { none() }
274-
275-
/** Gets a string matched by this regular expression. */
276-
string getAMatch() { none() }
277274
}
278275

279276
/**
@@ -458,20 +455,6 @@ class RegExpSequence extends RegExpTerm, TRegExpSequence {
458455
override predicate isNullable() {
459456
forall(RegExpTerm child | child = this.getAChild() | child.isNullable())
460457
}
461-
462-
// Why can't we use concat(...) with language[monotonicAggregates] here instead?
463-
override string getAMatch() { result = this.getAMatchFromChildAtIndex(0) }
464-
465-
private string getAMatchFromChildAtIndex(int i) {
466-
i = this.getNumChild() and result = ""
467-
or
468-
exists(string substring, string rest |
469-
substring = this.getChild(i).getAMatch() and
470-
rest = this.getAMatchFromChildAtIndex(i + 1)
471-
|
472-
result = substring + rest
473-
)
474-
}
475458
}
476459

477460
pragma[nomagic]
@@ -703,8 +686,6 @@ class RegExpCharacterClass extends RegExpTerm, TRegExpCharacterClass {
703686
override string getAPrimaryQlClass() { result = "RegExpCharacterClass" }
704687

705688
override predicate isNullable() { none() }
706-
707-
override string getAMatch() { not this.isInverted() and result = this.getAChild().getAMatch() }
708689
}
709690

710691
/**
@@ -819,8 +800,6 @@ class RegExpConstant extends RegExpTerm {
819800
override string getAPrimaryQlClass() { result = "RegExpConstant" }
820801

821802
override predicate isNullable() { none() }
822-
823-
override string getAMatch() { result = this.getValue() }
824803
}
825804

826805
/**
@@ -870,8 +849,6 @@ class RegExpGroup extends RegExpTerm, TRegExpGroup {
870849
override string getAPrimaryQlClass() { result = "RegExpGroup" }
871850

872851
override predicate isNullable() { this.getAChild().isNullable() }
873-
874-
override string getAMatch() { result = this.getAChild().getAMatch() }
875852
}
876853

877854
/**

ruby/ql/lib/codeql/ruby/security/IncompleteMultiCharacterSanitizationQuery.qll

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ pragma[noinline]
6060
private DangerousPrefixSubstring getADangerousMatchedChar(EmptyReplaceRegExpTerm t) {
6161
t.isNullable() and result = ""
6262
or
63-
result = t.getAMatch()
63+
result = t.getAMatchedString()
6464
or
6565
// A substring matched by some character class. This is only used to match the "word" part of a HTML tag (e.g. "iframe" in "<iframe").
6666
exists(ReDoSUtil::CharacterClass cc |
@@ -123,14 +123,14 @@ private predicate matchesDangerousPrefix(EmptyReplaceRegExpTerm t, string prefix
123123
kind = "path injection" and
124124
prefix = ["/..", "../"] and
125125
// If the regex is matching explicit path components, it is unlikely that it's being used as a sanitizer.
126-
not t.getSuccessor*().getAMatch().regexpMatch("(?is).*[a-z0-9_-].*")
126+
not t.getSuccessor*().getAMatchedString().regexpMatch("(?is).*[a-z0-9_-].*")
127127
or
128128
kind = "HTML element injection" and
129129
(
130130
// comments
131131
prefix = "<!--" and
132132
// If the regex is matching explicit textual content of an HTML comment, it is unlikely that it's being used as a sanitizer.
133-
not t.getSuccessor*().getAMatch().regexpMatch("(?is).*[a-z0-9_].*")
133+
not t.getSuccessor*().getAMatchedString().regexpMatch("(?is).*[a-z0-9_].*")
134134
or
135135
// specific tags
136136
// the `cript|scrip` case has been observed in the wild several times
@@ -148,11 +148,11 @@ private predicate matchesDangerousPrefix(EmptyReplaceRegExpTerm t, string prefix
148148
] and
149149
(
150150
// explicit matching: `onclick` and `ng-bind`
151-
t.getAMatch().regexpMatch("(?i)" + prefix + "[a-z]+")
151+
t.getAMatchedString().regexpMatch("(?i)" + prefix + "[a-z]+")
152152
or
153153
// regexp-based matching: `on[a-z]+`
154154
exists(EmptyReplaceRegExpTerm start | start = t.getAChild() |
155-
start.getAMatch().regexpMatch("(?i)[^a-z]*" + prefix) and
155+
start.getAMatchedString().regexpMatch("(?i)[^a-z]*" + prefix) and
156156
isCommonWordMatcher(start.getSuccessor())
157157
)
158158
)

0 commit comments

Comments
 (0)