fix: memoize css.recursiveCheck to remove exponential-time CSS DoS
Shorthand style property handlers (FontHandler and roughly a dozen siblings) tokenize a CSS value and feed it to css.recursiveCheck, which searches for a way to partition the tokens so every group is accepted by one of a handful of validator functions. The search was unmemoized: every recursive call is made on a suffix of the token slice, and the same suffix was re-explored once per partition path that reached it. For values with several tokens that more than one validator accepts on their own (e.g. runs of whitespace, which FontFamilyHandler treats as a valid empty entry), the number of such paths grows combinatorially with the number of tokens, making wall-clock cost roughly double per extra token. A short, attacker-controlled style value can hang a server sanitizing untrusted HTML. Memoize recursiveCheck on the length of the remaining token suffix: distinct suffixes of a given slice always have distinct lengths, and the outcome for a suffix depends only on its length within one top-level call (the validator set is constant throughout), so this caches each of at most len(value) distinct subproblems exactly once. This does not change any validator's behavior or which values are accepted or rejected, only how many times each subproblem is solved, turning the exponential search into a polynomial one. Closes #225
This commit is contained in:
1 parent
11fba4f573
commit
eb1d9c9bec
2 files changed
+154
-3
No files matched your search
+33
-3
@@ -337,16 +337,46 @@ func multiSplit(value string, seps ...string) []string {
|
||||
return curArray
|
||||
}
|
||||
|
||||
// recursiveCheck reports whether value can be fully consumed by
|
||||
// partitioning it, in order, into one or more contiguous groups of
|
||||
// tokens such that every group is accepted by at least one function in
|
||||
// funcs.
|
||||
//
|
||||
// The search is memoized on the length of the remaining suffix. Every
|
||||
// recursive call is made on value[i+1:], which is always a suffix of
|
||||
// the slice originally passed in, and since funcs is constant across
|
||||
// the whole search, the outcome for a given suffix depends only on how
|
||||
// many tokens remain in it. Without caching that outcome, the same
|
||||
// suffix can be re-explored once for every distinct partition path that
|
||||
// reaches it, which is exponential in len(value) whenever a value has
|
||||
// several tokens that multiple funcs accept on their own (for example
|
||||
// runs of whitespace, which some handlers treat as a valid empty
|
||||
// entry). Caching means each of the at most len(value) distinct
|
||||
// suffixes is solved once, turning that exponential search into a
|
||||
// polynomial one without changing which values are ultimately accepted
|
||||
// or rejected.
|
||||
func recursiveCheck(value []string, funcs []func(string) bool) bool {
|
||||
memo := make(map[int]bool, len(value))
|
||||
return recursiveCheckMemo(value, funcs, memo)
|
||||
}
|
||||
|
||||
func recursiveCheckMemo(value []string, funcs []func(string) bool, memo map[int]bool) bool {
|
||||
if cached, ok := memo[len(value)]; ok {
|
||||
return cached
|
||||
}
|
||||
result := false
|
||||
outer:
|
||||
for i := 0; i < len(value); i++ {
|
||||
tempVal := strings.Join(value[:i+1], " ")
|
||||
for _, j := range funcs {
|
||||
if j(tempVal) && (len(value[i+1:]) == 0 || recursiveCheck(value[i+1:], funcs)) {
|
||||
return true
|
||||
if j(tempVal) && (len(value[i+1:]) == 0 || recursiveCheckMemo(value[i+1:], funcs, memo)) {
|
||||
result = true
|
||||
break outer
|
||||
}
|
||||
}
|
||||
}
|
||||
return false
|
||||
memo[len(value)] = result
|
||||
return result
|
||||
}
|
||||
|
||||
func in(value []string, arr []string) bool {
|
||||
|
||||
Reference in new issue
Block a user