Commit
The escaping of attributes should have been handled according to https://html.spec.whatwg.org/multipage/parsing.html#escapingString and as it was not, it allowed the possibility of an XSS by overloading a known attribute like the href. This can be seen in the test within this commit and is recognised as a vulnerability that existed prior to this commit. Additional update to the versions of modules we depend upon
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,8 @@ module github.com/microcosm-cc/bluemonday | |
| go 1.16 | ||
|
|
||
| require ( | ||
| github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong. 
grafana-dee
Contributor
|
||
| github.com/aymerick/douceur v0.2.0 | ||
| github.com/gorilla/css v1.0.0 // indirect | ||
| golang.org/x/net v0.0.0-20210614182718-04defd469f4e | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -130,14 +130,18 @@ func escapeUrlComponent(w stringWriterWriter, val string) error { | |
| return err | ||
| } | ||
|
|
||
| // Query represents a single part of the query string, a query param | ||
| type Query struct { | ||
| Key string | ||
| Value string | ||
| HasValue bool | ||
| } | ||
|
|
||
| func parseQuery(query string) (values []Query, err error) { | ||
| // This is essentially a copy of parseQuery from | ||
| // https://golang.org/src/net/url/url.go but adjusted to build our values | ||
| // based on our type, which we need to preserve the ordering of the query | ||
| // string | ||
| for query != "" { | ||
| key := query | ||
| if i := strings.IndexAny(key, "&;"); i >= 0 { | ||
|
|
@@ -213,43 +217,6 @@ func sanitizedURL(val string) (string, error) { | |
| return u.String(), nil | ||
| } | ||
|
|
||
| // Performs the actual sanitization process. | ||
| func (p *Policy) sanitizeWithBuff(r io.Reader) *bytes.Buffer { | ||
| var buff bytes.Buffer | ||
|
|
@@ -344,7 +311,9 @@ func (p *Policy) sanitize(r io.Reader, w io.Writer) error { | |
| aps = aa | ||
| } | ||
| if len(token.Attr) != 0 { | ||
| token.Attr = escapeAttributes( | ||
| p.sanitizeAttrs(token.Data, token.Attr, aps), | ||
| ) | ||
This comment has been minimized.
Sorry, something went wrong.
This comment has been minimized.
Sorry, something went wrong.
grafana-dee
Contributor
|
||
| } | ||
|
|
||
| if len(token.Attr) == 0 { | ||
|
|
@@ -361,15 +330,8 @@ func (p *Policy) sanitize(r io.Reader, w io.Writer) error { | |
| } | ||
|
|
||
| if !skipElementContent { | ||
| if _, err := buff.WriteString(token.String()); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -439,7 +401,7 @@ func (p *Policy) sanitize(r io.Reader, w io.Writer) error { | |
| } | ||
|
|
||
| if len(token.Attr) != 0 { | ||
| token.Attr = escapeAttributes(p.sanitizeAttrs(token.Data, token.Attr, aps)) | ||
| } | ||
|
|
||
| if len(token.Attr) == 0 && !p.allowNoAttrs(token.Data) { | ||
|
|
@@ -451,15 +413,8 @@ func (p *Policy) sanitize(r io.Reader, w io.Writer) error { | |
| } | ||
| } | ||
| if !skipElementContent { | ||
| if _, err := buff.WriteString(token.String()); err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -569,9 +524,11 @@ attrsLoop: | |
| for _, ap := range apl { | ||
| if ap.regexp != nil { | ||
| if ap.regexp.MatchString(htmlAttr.Val) { | ||
| htmlAttr.Val = escapeAttribute(htmlAttr.Val) | ||
| cleanAttrs = append(cleanAttrs, htmlAttr) | ||
| } | ||
| } else { | ||
| htmlAttr.Val = escapeAttribute(htmlAttr.Val) | ||
| cleanAttrs = append(cleanAttrs, htmlAttr) | ||
| } | ||
| } | ||
|
|
@@ -1087,3 +1044,18 @@ func normaliseElementName(str string) string { | |
| `"`, | ||
| ) | ||
| } | ||
|
|
||
| func escapeAttributes(attrs []html.Attribute) []html.Attribute { | ||
| escapedAttrs := []html.Attribute{} | ||
| for _, attr := range attrs { | ||
| attr.Val = escapeAttribute(attr.Val) | ||
| escapedAttrs = append(escapedAttrs, attr) | ||
| } | ||
| return escapedAttrs | ||
| } | ||
|
|
||
| func escapeAttribute(val string) string { | ||
| val = strings.Replace(val, string([]rune{'\u00A0'}), ` `, -1) | ||
| val = strings.Replace(val, `"`, `"`, -1) | ||
| return val | ||
| } | ||
Did you intend to use this somewhere? Running
go mod tidywould remove this and clean up some old entries ingo.sum.