Skip to content

Commit

Permalink
fix: XSS issues in page descriptions (#307)
Browse files Browse the repository at this point in the history
  • Loading branch information
Brookke authored Apr 18, 2024
1 parent 767a89e commit 4304de1
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 70 deletions.
2 changes: 2 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ require (
github.com/codegangsta/negroni v1.0.0
github.com/gedex/inflector v0.0.0-20170307190818-16278e9db813
github.com/gorilla/mux v1.8.0
github.com/grokify/html-strip-tags-go v0.0.1
github.com/microcosm-cc/bluemonday v1.0.15
github.com/stretchr/graceful v1.2.15
golang.org/x/net v0.7.0
)
13 changes: 12 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,21 @@ github.com/BurntSushi/toml v0.4.1 h1:GaI7EiDXDRfa8VshkTj7Fym7ha+y8/XxIgD2okUIjLw
github.com/BurntSushi/toml v0.4.1/go.mod h1:CxXYINrC8qIiEnFrOxCa7Jy5BFHlXnUU2pbicEuybxQ=
github.com/UniversityRadioYork/myradio-go v0.0.0-20210821190257-67bde48f2e7e h1:4tEgYVcZraVZAVMaV/lTCPNiW4ZE3OASjV4JKw3KkXc=
github.com/UniversityRadioYork/myradio-go v0.0.0-20210821190257-67bde48f2e7e/go.mod h1:iFH6u3KFaQ73MR9bfqTThGd7TFUYUe9cAajxxh9E0Z8=
github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d/go.mod h1:WaHUgvxTVq04UNunO+XhnAqY/wQc+bxr74GqbsZ/Jqw=
github.com/aymerick/douceur v0.2.0 h1:Mv+mAeH1Q+n9Fr+oyamOlAkUNPWPlA8PPGR0QAaYuPk=
github.com/aymerick/douceur v0.2.0/go.mod h1:wlT5vV2O3h55X9m7iVYN0TBM0NH/MmbLnd30/FjWUq4=
github.com/codegangsta/negroni v1.0.0 h1:+aYywywx4bnKXWvoWtRfJ91vC59NbEhEY03sZjQhbVY=
github.com/codegangsta/negroni v1.0.0/go.mod h1:v0y3T5G7Y1UlFfyxFn/QLRU4a2EuNau2iZY63YTKWo0=
github.com/gedex/inflector v0.0.0-20170307190818-16278e9db813 h1:Uc+IZ7gYqAf/rSGFplbWBSHaGolEQlNLgMgSE3ccnIQ=
github.com/gedex/inflector v0.0.0-20170307190818-16278e9db813/go.mod h1:P+oSoE9yhSRvsmYyZsshflcR6ePWYLql6UU1amW13IM=
github.com/gorilla/css v1.0.0 h1:BQqNyPTi50JCFMTw/b67hByjMVXZRwGha6wxVGkeihY=
github.com/gorilla/css v1.0.0/go.mod h1:Dn721qIggHpt4+EFCcTLTU/vk5ySda2ReITrtgBl60c=
github.com/gorilla/mux v1.8.0 h1:i40aqfkR1h2SlN9hojwV5ZA91wcXFOvkdNIeFDP5koI=
github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
github.com/grokify/html-strip-tags-go v0.0.1 h1:0fThFwLbW7P/kOiTBs03FsJSV9RM2M/Q/MOnCQxKMo0=
github.com/grokify/html-strip-tags-go v0.0.1/go.mod h1:2Su6romC5/1VXOQMaWL2yb618ARB8iVo6/DR99A6d78=
github.com/microcosm-cc/bluemonday v1.0.15 h1:J4uN+qPng9rvkBZBoBb8YGR+ijuklIMpSOZZLjYpbeY=
github.com/microcosm-cc/bluemonday v1.0.15/go.mod h1:ZLvAzeakRwrGnzQEvstVzVt3ZpqOF2+sdFr0Om+ce30=
github.com/stretchr/graceful v1.2.15 h1:vmXbwPGfe8bI6KkgmHry/P1Pk63bM3TDcfi+5mh+VHg=
github.com/stretchr/graceful v1.2.15/go.mod h1:IxdGAOTZueMKoBr3oJIzdeg5CCCXbHXfV44sLhfAXXI=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
Expand All @@ -16,13 +25,15 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20210614182718-04defd469f4e/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.7.0 h1:rJrUqqhjsgNp7KqAIc25s9pZnjU7TUcSY7HcVZjdn1g=
golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
Expand All @@ -32,8 +43,8 @@ golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuX
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.7.0 h1:4BRB4x83lYWy72KwLD/qYDuTu7q9PjSagHvijDw7cLo=
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
Expand Down
57 changes: 5 additions & 52 deletions utils/html.go
Original file line number Diff line number Diff line change
@@ -1,62 +1,15 @@
package utils

import (
"bytes"
"strings"
"unicode"

"golang.org/x/net/html"
"github.com/microcosm-cc/bluemonday"
)

// This is non-exhaustive, but captures most required nodes
const stripAllowedNodes = "html body p a strong em span section div"

// StripHTML strips HTML tags from a string, extracting all plain text.
func StripHTML(htmls string) (string, error) {
doc, err := html.Parse(strings.NewReader(htmls))
if err != nil {
return "", err
}

var buffer bytes.Buffer
anodes := strings.Split(stripAllowedNodes, " ")

var f func(n *html.Node)
f = func(n *html.Node) {
switch n.Type {
case html.DocumentNode:
if n.FirstChild != nil {
f(n.FirstChild)
}
case html.ElementNode:
// Decide whether to descend into the element's
// children
allowed := false
for _, anode := range anodes {
if n.Data == anode {
allowed = true
break
}
}
func StripHTML(htmls string) string {
sanitizer := bluemonday.StrictPolicy()

if allowed && n.FirstChild != nil {
f(n.FirstChild)
}
return sanitizer.Sanitize(htmls)

// Ensure whitespace between paragraphs (hack!)
if n.Data == "p" {
buffer.WriteString("\n\n")
}
case html.TextNode:
buffer.WriteString(n.Data)
}

if n.NextSibling != nil {
f(n.NextSibling)
}
}
}

f(doc)

return strings.TrimFunc(buffer.String(), unicode.IsSpace), nil
}
11 changes: 5 additions & 6 deletions utils/html_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ func TestStripHTML(t *testing.T) {
Input: "raw test",
},
{
Expected: "1\n\n2",
Input: "<p>1</p><p>2</p>",
Expected: "1\n2",
Input: "<p>1</p>\n<p>2</p>",
},

{
Expand All @@ -32,10 +32,9 @@ func TestStripHTML(t *testing.T) {
}

for _, c := range cases {
got, err := utils.StripHTML(c.Input)
if err != nil {
t.Error(err)
} else if c.Expected != got {
got := utils.StripHTML(c.Input)

if c.Expected != got {
t.Errorf("expected:\n%s\n\ngot:\n%s", c.Expected, got)
}
}
Expand Down
16 changes: 6 additions & 10 deletions utils/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (

"github.com/UniversityRadioYork/2016-site/structs"
"github.com/UniversityRadioYork/myradio-go"
"github.com/microcosm-cc/bluemonday"
)

// TemplatePrefix is the constant containing the filepath prefix for templates.
Expand Down Expand Up @@ -127,14 +128,6 @@ func RenderTemplate(w http.ResponseWriter, context structs.PageContext, data int
}
return a.Sub(b), nil
},
// TODO(CaptainHayashi): this is temporary
"stripHTML": func(s string) string {
d, err := StripHTML(s)
if err != nil {
return "Error stripping HTML"
}
return d
},
"week": FormatWeekRelative,
"plural": inflector.Pluralize,
})
Expand All @@ -149,6 +142,9 @@ func RenderTemplate(w http.ResponseWriter, context structs.PageContext, data int
// renderHTML takes some html as a string and returns a template.HTML
//
// Handles plain text gracefully.
func renderHTML(value interface{}) template.HTML {
return template.HTML(fmt.Sprint(value))
func renderHTML(value string) template.HTML {
sanitizer := bluemonday.UGCPolicy()
sanitizedString := sanitizer.Sanitize(value)

return template.HTML(sanitizedString)
}
2 changes: 1 addition & 1 deletion views/schedule_week.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@
<tr>
<td class="time-header mobile-only hour-{{.Hour | printf "%02d"}}">{{.Hour | printf "%02d"}}:{{.Minute | printf "%02d"}}</td>
{{if ne .RowSpan 0}}
<td rowspan="{{.RowSpan}}" class="schedule-timeslot schedule-block-{{.Item.Block}}" title="{{stripHTML .Item.Desc}}"
<td rowspan="{{.RowSpan}}" class="schedule-timeslot schedule-block-{{.Item.Block}}"
{{if .Item.PageURL}} onclick="window.location.href='{{url .Item.PageURL}}'" {{end}} >
{{if .Item.PageURL}}
<a href="{{url .Item.PageURL}}" >{{.Item.Name}}</a>
Expand Down

0 comments on commit 4304de1

Please sign in to comment.