mirror of
https://github.com/github/codeql.git
synced 2026-04-28 18:25:24 +02:00
Js/Ruby: Share IncompleteHostnameRegExp.ql
This commit is contained in:
@@ -398,6 +398,8 @@ class RegExpAlt extends RegExpTerm, TRegExpAlt {
|
||||
override string getAPrimaryQlClass() { result = "RegExpAlt" }
|
||||
}
|
||||
|
||||
class RegExpCharEscape = RegExpEscape;
|
||||
|
||||
class RegExpEscape extends RegExpNormalChar {
|
||||
RegExpEscape() { re.escapedCharacter(start, end) }
|
||||
|
||||
|
||||
@@ -3,8 +3,7 @@
|
||||
* that match URLs and hostname patterns.
|
||||
*/
|
||||
|
||||
private import codeql.ruby.security.performance.RegExpTreeView
|
||||
private import codeql.ruby.DataFlow
|
||||
private import HostnameRegexpSpecific
|
||||
|
||||
/**
|
||||
* Holds if the given constant is unlikely to occur in the origin part of a URL.
|
||||
@@ -20,7 +19,7 @@ predicate isConstantInvalidInsideOrigin(RegExpConstant term) {
|
||||
|
||||
/** Holds if `term` is a dot constant of form `\.` or `[.]`. */
|
||||
predicate isDotConstant(RegExpTerm term) {
|
||||
term.(RegExpEscape).getValue() = "."
|
||||
term.(RegExpCharEscape).getValue() = "."
|
||||
or
|
||||
exists(RegExpCharacterClass cls |
|
||||
term = cls and
|
||||
@@ -108,3 +107,96 @@ predicate alwaysMatchesHostnameAlt(RegExpAlt alt, int i) {
|
||||
alwaysMatchesHostnameAlt(alt, i - 1) and
|
||||
alwaysMatchesHostname(alt.getChild(i))
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `term` occurs inside a quantifier or alternative (and thus
|
||||
* can not be expected to correspond to a unique match), or as part of
|
||||
* a lookaround assertion (which are rarely used for capture groups).
|
||||
*/
|
||||
predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
|
||||
exists(RegExpParent parent | parent = term.getParent() |
|
||||
parent instanceof RegExpAlt
|
||||
or
|
||||
parent instanceof RegExpQuantifier
|
||||
or
|
||||
parent instanceof RegExpSubPattern
|
||||
or
|
||||
isInsideChoiceOrSubPattern(parent)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `group` is likely to be used as a capture group.
|
||||
*/
|
||||
predicate isLikelyCaptureGroup(RegExpGroup group) {
|
||||
group.isCapture() and
|
||||
not isInsideChoiceOrSubPattern(group)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `seq` contains two consecutive dots `..` or escaped dots.
|
||||
*
|
||||
* At least one of these dots is not intended to be a subdomain separator,
|
||||
* so we avoid flagging the pattern in this case.
|
||||
*/
|
||||
predicate hasConsecutiveDots(RegExpSequence seq) {
|
||||
exists(int i |
|
||||
isDotLike(seq.getChild(i)) and
|
||||
isDotLike(seq.getChild(i + 1))
|
||||
)
|
||||
}
|
||||
|
||||
predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
|
||||
seq = regexp.getAChild*() and
|
||||
exists(RegExpDot unescapedDot, int i, string hostname |
|
||||
hasTopLevelDomainEnding(seq, i) and
|
||||
not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
|
||||
not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
|
||||
unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
|
||||
unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
|
||||
not hasConsecutiveDots(unescapedDot.getParent()) and
|
||||
hostname =
|
||||
seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
|
||||
seq.getChild(i).getRawValue()
|
||||
|
|
||||
if unescapedDot.getParent() instanceof RegExpQuantifier
|
||||
then
|
||||
// `.*\.example.com` can match `evil.com/?x=.example.com`
|
||||
//
|
||||
// This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
|
||||
// We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
|
||||
// Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
|
||||
// and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
|
||||
seq.getChild(0) instanceof RegExpCaret and
|
||||
not seq.getAChild() instanceof RegExpDollar and
|
||||
seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
|
||||
msg =
|
||||
"has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
|
||||
+ "' which may cause '" + hostname +
|
||||
"' to be matched anywhere in the URL, outside the hostname."
|
||||
else
|
||||
msg =
|
||||
"has an unescaped '.' before '" + hostname +
|
||||
"', so it might match more hosts than expected."
|
||||
)
|
||||
}
|
||||
|
||||
predicate incompleteHostnameRegExp(
|
||||
RegExpSequence hostSequence, string message, DataFlow::Node aux, string label
|
||||
) {
|
||||
exists(RegExpPatternSource re, RegExpTerm regexp, string msg, string kind |
|
||||
regexp = re.getRegExpTerm() and
|
||||
isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
|
||||
(
|
||||
if re.getAParse() != re
|
||||
then (
|
||||
kind = "string, which is used as a regular expression $@," and
|
||||
aux = re.getAParse()
|
||||
) else (
|
||||
kind = "regular expression" and aux = re
|
||||
)
|
||||
)
|
||||
|
|
||||
message = "This " + kind + " " + msg and label = "here"
|
||||
)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,2 @@
|
||||
import codeql.ruby.security.performance.RegExpTreeView
|
||||
import codeql.ruby.DataFlow
|
||||
@@ -12,96 +12,5 @@
|
||||
*/
|
||||
|
||||
import HostnameRegexpShared
|
||||
import codeql.ruby.security.performance.RegExpTreeView
|
||||
import codeql.ruby.DataFlow
|
||||
import codeql.ruby.AST as AST
|
||||
|
||||
/**
|
||||
* Holds if `term` occurs inside a quantifier or alternative (and thus
|
||||
* can not be expected to correspond to a unique match), or as part of
|
||||
* a lookaround assertion (which are rarely used for capture groups).
|
||||
*/
|
||||
predicate isInsideChoiceOrSubPattern(RegExpTerm term) {
|
||||
exists(RegExpParent parent | parent = term.getParent() |
|
||||
parent instanceof RegExpAlt
|
||||
or
|
||||
parent instanceof RegExpQuantifier
|
||||
or
|
||||
parent instanceof RegExpSubPattern
|
||||
or
|
||||
isInsideChoiceOrSubPattern(parent)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `group` is likely to be used as a capture group.
|
||||
*/
|
||||
predicate isLikelyCaptureGroup(RegExpGroup group) {
|
||||
group.isCapture() and
|
||||
not isInsideChoiceOrSubPattern(group)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if `seq` contains two consecutive dots `..` or escaped dots.
|
||||
*
|
||||
* At least one of these dots is not intended to be a subdomain separator,
|
||||
* so we avoid flagging the pattern in this case.
|
||||
*/
|
||||
predicate hasConsecutiveDots(RegExpSequence seq) {
|
||||
exists(int i |
|
||||
isDotLike(seq.getChild(i)) and
|
||||
isDotLike(seq.getChild(i + 1))
|
||||
)
|
||||
}
|
||||
|
||||
predicate isIncompleteHostNameRegExpPattern(RegExpTerm regexp, RegExpSequence seq, string msg) {
|
||||
seq = regexp.getAChild*() and
|
||||
exists(RegExpDot unescapedDot, int i, string hostname |
|
||||
hasTopLevelDomainEnding(seq, i) and
|
||||
not isConstantInvalidInsideOrigin(seq.getChild([0 .. i - 1]).getAChild*()) and
|
||||
not isLikelyCaptureGroup(seq.getChild([i .. seq.getNumChild() - 1]).getAChild*()) and
|
||||
unescapedDot = seq.getChild([0 .. i - 1]).getAChild*() and
|
||||
unescapedDot != seq.getChild(i - 1) and // Should not be the '.' immediately before the TLD
|
||||
not hasConsecutiveDots(unescapedDot.getParent()) and
|
||||
hostname =
|
||||
seq.getChild(i - 2).getRawValue() + seq.getChild(i - 1).getRawValue() +
|
||||
seq.getChild(i).getRawValue()
|
||||
|
|
||||
if unescapedDot.getParent() instanceof RegExpQuantifier
|
||||
then
|
||||
// `.*\.example.com` can match `evil.com/?x=.example.com`
|
||||
//
|
||||
// This problem only occurs when the pattern is applied against a full URL, not just a hostname/origin.
|
||||
// We therefore check if the pattern includes a suffix after the TLD, such as `.*\.example.com/`.
|
||||
// Note that a post-anchored pattern (`.*\.example.com$`) will usually fail to match a full URL,
|
||||
// and patterns with neither a suffix nor an anchor fall under the purview of MissingRegExpAnchor.
|
||||
seq.getChild(0) instanceof RegExpCaret and
|
||||
not seq.getAChild() instanceof RegExpDollar and
|
||||
seq.getChild([i .. i + 1]).(RegExpConstant).getValue().regexpMatch(".*[/?#].*") and
|
||||
msg =
|
||||
"has an unrestricted wildcard '" + unescapedDot.getParent().(RegExpQuantifier).getRawValue()
|
||||
+ "' which may cause '" + hostname +
|
||||
"' to be matched anywhere in the URL, outside the hostname."
|
||||
else
|
||||
msg =
|
||||
"has an unescaped '.' before '" + hostname +
|
||||
"', so it might match more hosts than expected."
|
||||
)
|
||||
}
|
||||
|
||||
from
|
||||
RegExpPatternSource re, RegExpTerm regexp, RegExpSequence hostSequence, string msg, string kind,
|
||||
DataFlow::Node aux
|
||||
where
|
||||
regexp = re.getRegExpTerm() and
|
||||
isIncompleteHostNameRegExpPattern(regexp, hostSequence, msg) and
|
||||
(
|
||||
if re.getAParse() != re
|
||||
then (
|
||||
kind = "string, which is used as a regular expression $@," and
|
||||
aux = re.getAParse()
|
||||
) else (
|
||||
kind = "regular expression" and aux = re
|
||||
)
|
||||
)
|
||||
select hostSequence, "This " + kind + " " + msg, aux, "here"
|
||||
query predicate problems = incompleteHostnameRegExp/4;
|
||||
|
||||
Reference in New Issue
Block a user