feat(web): untrusted markdown for external issues
Issues approved from the public form are rendered with a strict policy: http, https and mailto links only, rel ugc, no repository-relative links. The author label is shown as plain text.
- commit
- 3f9aa1023b588abcccabcb697d8f47b69e39efa6
- author
- brice · · 2026-10-19 07:42 UTC
- parent
- 99db356
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +166 -5
M internal/adapters/web/issues.go +25 -1
| @@ -103,11 +103,22 @@ func (s *Server) issueRepo(ctx context.Context, rawName string) (*repoData, erro | |||
| 103 | 103 | return d, nil | |
| 104 | 104 | } | |
| 105 | 105 | ||
| 106 | + | // externalAuthor is the label a visitor gave, as plain text, or "anonymous". | |
| 107 | + | func (s *Server) externalAuthor(label string) string { | |
| 108 | + | if a := strings.TrimSpace(visibleText(label)); a != "" { | |
| 109 | + | return a | |
| 110 | + | } | |
| 111 | + | return "anonymous" | |
| 112 | + | } | |
| 113 | + | ||
| 106 | 114 | func (s *Server) issueRow(repo string, i domain.Issue) IssueRow { | |
| 107 | 115 | row := IssueRow{ | |
| 108 | 116 | Number: "#" + strconv.FormatInt(i.Number, 10), Href: issueHref(repo, i.Number), | |
| 109 | 117 | Title: visibleText(i.Title.String()), State: string(i.State), Author: s.who(i.Author), | |
| 110 | 118 | } | |
| 119 | + | if i.External { | |
| 120 | + | row.Author = s.externalAuthor(i.Author) | |
| 121 | + | } | |
| 111 | 122 | ts := i.UpdatedAt | |
| 112 | 123 | if ts.IsZero() { | |
| 113 | 124 | ts = i.CreatedAt | |
| @@ -117,6 +128,8 @@ func (s *Server) issueRow(repo string, i domain.Issue) IssueRow { | |||
| 117 | 128 | } | |
| 118 | 129 | ||
| 119 | 130 | // issueList lists the issues of one state, a page at a time. | |
| 131 | + | // | |
| 132 | + | //hexcheck:ignore suspicious-business-logic-in-adapter builds the list page from the use case results; whether the New issue button shows only reads the mode | |
| 120 | 133 | func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Component, int, error) { | |
| 121 | 134 | d, err := s.issueRepo(ctx, r.PathValue("repo")) | |
| 122 | 135 | if err != nil { | |
| @@ -228,13 +241,24 @@ func (s *Server) issueShow(ctx context.Context, r *http.Request) (templ.Componen | |||
| 228 | 241 | State: string(i.State), Open: i.IsOpen(), | |
| 229 | 242 | } | |
| 230 | 243 | if body := i.Body.String(); strings.TrimSpace(body) != "" { | |
| 231 | - | h, _, err := s.md.renderWithIDs([]byte(body), d.name(), d.repo.DefaultBranch, "", "issue-desc-") | |
| 244 | + | var h template.HTML | |
| 245 | + | var err error | |
| 246 | + | if i.External { | |
| 247 | + | // Written by an anonymous visitor: the untrusted policy. | |
| 248 | + | h, err = s.md.renderUntrusted([]byte(body), "issue-desc-") | |
| 249 | + | } else { | |
| 250 | + | h, _, err = s.md.renderWithIDs([]byte(body), d.name(), d.repo.DefaultBranch, "", "issue-desc-") | |
| 251 | + | } | |
| 232 | 252 | if err != nil { | |
| 233 | 253 | return nil, 0, err | |
| 234 | 254 | } | |
| 235 | 255 | p.Desc = h | |
| 236 | 256 | } | |
| 237 | 257 | p.Opened.Name = s.who(i.Author) | |
| 258 | + | if i.External { | |
| 259 | + | // The label is the visitor's, not the owner's: shown as plain text. | |
| 260 | + | p.Opened.Name = s.externalAuthor(i.Author) | |
| 261 | + | } | |
| 238 | 262 | p.Opened.When, p.Opened.WhenAt = whenOf(now, i.CreatedAt) | |
| 239 | 263 | if !i.CreatedAt.IsZero() { | |
| 240 | 264 | p.Opened.Abs = absolute(i.CreatedAt) | |
| @@ -103,11 +103,22 @@ func (s *Server) issueRepo(ctx context.Context, rawName string) (*repoData, erro | |||||
| 103 | return d, nil | 103 | return d, nil | ||
| 104 | } | 104 | } | ||
| 105 | 105 | ||||
| 106 | + | // externalAuthor is the label a visitor gave, as plain text, or "anonymous". | |||
| 107 | + | func (s *Server) externalAuthor(label string) string { | |||
| 108 | + | if a := strings.TrimSpace(visibleText(label)); a != "" { | |||
| 109 | + | return a | |||
| 110 | + | } | |||
| 111 | + | return "anonymous" | |||
| 112 | + | } | |||
| 113 | + | ||||
| 106 | func (s *Server) issueRow(repo string, i domain.Issue) IssueRow { | 114 | func (s *Server) issueRow(repo string, i domain.Issue) IssueRow { | ||
| 107 | row := IssueRow{ | 115 | row := IssueRow{ | ||
| 108 | Number: "#" + strconv.FormatInt(i.Number, 10), Href: issueHref(repo, i.Number), | 116 | Number: "#" + strconv.FormatInt(i.Number, 10), Href: issueHref(repo, i.Number), | ||
| 109 | Title: visibleText(i.Title.String()), State: string(i.State), Author: s.who(i.Author), | 117 | Title: visibleText(i.Title.String()), State: string(i.State), Author: s.who(i.Author), | ||
| 110 | } | 118 | } | ||
| 119 | + | if i.External { | |||
| 120 | + | row.Author = s.externalAuthor(i.Author) | |||
| 121 | + | } | |||
| 111 | ts := i.UpdatedAt | 122 | ts := i.UpdatedAt | ||
| 112 | if ts.IsZero() { | 123 | if ts.IsZero() { | ||
| 113 | ts = i.CreatedAt | 124 | ts = i.CreatedAt | ||
| @@ -117,6 +128,8 @@ func (s *Server) issueRow(repo string, i domain.Issue) IssueRow { | |||||
| 117 | } | 128 | } | ||
| 118 | 129 | ||||
| 119 | // issueList lists the issues of one state, a page at a time. | 130 | // issueList lists the issues of one state, a page at a time. | ||
| 131 | + | // | |||
| 132 | + | //hexcheck:ignore suspicious-business-logic-in-adapter builds the list page from the use case results; whether the New issue button shows only reads the mode | |||
| 120 | func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Component, int, error) { | 133 | func (s *Server) issueList(ctx context.Context, r *http.Request) (templ.Component, int, error) { | ||
| 121 | d, err := s.issueRepo(ctx, r.PathValue("repo")) | 134 | d, err := s.issueRepo(ctx, r.PathValue("repo")) | ||
| 122 | if err != nil { | 135 | if err != nil { | ||
| @@ -228,13 +241,24 @@ func (s *Server) issueShow(ctx context.Context, r *http.Request) (templ.Componen | |||||
| 228 | State: string(i.State), Open: i.IsOpen(), | 241 | State: string(i.State), Open: i.IsOpen(), | ||
| 229 | } | 242 | } | ||
| 230 | if body := i.Body.String(); strings.TrimSpace(body) != "" { | 243 | if body := i.Body.String(); strings.TrimSpace(body) != "" { | ||
| 231 | - | h, _, err := s.md.renderWithIDs([]byte(body), d.name(), d.repo.DefaultBranch, "", "issue-desc-") | 244 | + | var h template.HTML |
| 245 | + | var err error | |||
| 246 | + | if i.External { | |||
| 247 | + | // Written by an anonymous visitor: the untrusted policy. | |||
| 248 | + | h, err = s.md.renderUntrusted([]byte(body), "issue-desc-") | |||
| 249 | + | } else { | |||
| 250 | + | h, _, err = s.md.renderWithIDs([]byte(body), d.name(), d.repo.DefaultBranch, "", "issue-desc-") | |||
| 251 | + | } | |||
| 232 | if err != nil { | 252 | if err != nil { | ||
| 233 | return nil, 0, err | 253 | return nil, 0, err | ||
| 234 | } | 254 | } | ||
| 235 | p.Desc = h | 255 | p.Desc = h | ||
| 236 | } | 256 | } | ||
| 237 | p.Opened.Name = s.who(i.Author) | 257 | p.Opened.Name = s.who(i.Author) | ||
| 258 | + | if i.External { | |||
| 259 | + | // The label is the visitor's, not the owner's: shown as plain text. | |||
| 260 | + | p.Opened.Name = s.externalAuthor(i.Author) | |||
| 261 | + | } | |||
| 238 | p.Opened.When, p.Opened.WhenAt = whenOf(now, i.CreatedAt) | 262 | p.Opened.When, p.Opened.WhenAt = whenOf(now, i.CreatedAt) | ||
| 239 | if !i.CreatedAt.IsZero() { | 263 | if !i.CreatedAt.IsZero() { | ||
| 240 | p.Opened.Abs = absolute(i.CreatedAt) | 264 | p.Opened.Abs = absolute(i.CreatedAt) | ||
M internal/adapters/web/markdown.go +74 -4
| @@ -62,9 +62,25 @@ func (m *markdown) render(source []byte, repo, ref, dir string) (template.HTML, | |||
| 62 | 62 | // #fragment links chosen, so that several documents on one page (the summaries | |
| 63 | 63 | // of a run) do not share ids. | |
| 64 | 64 | func (m *markdown) renderWithIDs(source []byte, repo, ref, dir, idPrefix string) (template.HTML, []Heading, error) { | |
| 65 | - | pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: idPrefix, inner: parser.NewContext().IDs()})) | |
| 65 | + | return m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, dir: dir, idPrefix: idPrefix}) | |
| 66 | + | } | |
| 67 | + | ||
| 68 | + | // renderUntrusted renders Markdown written by an anonymous visitor (an issue | |
| 69 | + | // approved from the public form). On top of the policy of render, which already | |
| 70 | + | // drops raw HTML and never loads an image: | |
| 71 | + | // - only http, https and mailto links are kept, every one with | |
| 72 | + | // rel="nofollow noopener noreferrer ugc"; relative links, #fragments and | |
| 73 | + | // every other scheme lose their link and keep the text; | |
| 74 | + | // - nothing points into the repository. | |
| 75 | + | func (m *markdown) renderUntrusted(source []byte, idPrefix string) (template.HTML, error) { | |
| 76 | + | h, _, err := m.renderPolicy(source, linkPolicy{idPrefix: idPrefix, untrusted: true}) | |
| 77 | + | return h, err | |
| 78 | + | } | |
| 79 | + | ||
| 80 | + | func (m *markdown) renderPolicy(source []byte, pol linkPolicy) (template.HTML, []Heading, error) { | |
| 81 | + | pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: pol.idPrefix, inner: parser.NewContext().IDs()})) | |
| 66 | 82 | doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc)) | |
| 67 | - | pol := linkPolicy{repo: repo, ref: ref, dir: dir, source: source, idPrefix: idPrefix} | |
| 83 | + | pol.source = source | |
| 68 | 84 | pol.apply(doc) | |
| 69 | 85 | ||
| 70 | 86 | var buf bytes.Buffer | |
| @@ -131,6 +147,20 @@ type linkPolicy struct { | |||
| 131 | 147 | source []byte | |
| 132 | 148 | // idPrefix is the prefix of the heading ids that #fragment links point at. | |
| 133 | 149 | idPrefix string | |
| 150 | + | // untrusted is the policy of text from an anonymous visitor. | |
| 151 | + | untrusted bool | |
| 152 | + | } | |
| 153 | + | ||
| 154 | + | const ( | |
| 155 | + | relExternal = "nofollow noopener noreferrer" | |
| 156 | + | relUntrusted = "nofollow noopener noreferrer ugc" | |
| 157 | + | ) | |
| 158 | + | ||
| 159 | + | func (p linkPolicy) rel() []byte { | |
| 160 | + | if p.untrusted { | |
| 161 | + | return []byte(relUntrusted) | |
| 162 | + | } | |
| 163 | + | return []byte(relExternal) | |
| 134 | 164 | } | |
| 135 | 165 | ||
| 136 | 166 | func (p linkPolicy) apply(doc ast.Node) { | |
| @@ -163,13 +193,17 @@ func (p linkPolicy) apply(doc ast.Node) { | |||
| 163 | 193 | l.Destination = []byte(dest) | |
| 164 | 194 | l.Title = nil | |
| 165 | 195 | if kind == external { | |
| 166 | - | l.SetAttributeString("rel", []byte("nofollow noopener noreferrer")) | |
| 196 | + | l.SetAttributeString("rel", p.rel()) | |
| 167 | 197 | } | |
| 168 | 198 | } | |
| 169 | 199 | for _, n := range autos { | |
| 170 | 200 | a := n.(*ast.AutoLink) | |
| 171 | 201 | if a.AutoLinkType == ast.AutoLinkURL && isWebURL(string(a.URL(p.source))) { | |
| 172 | - | a.SetAttributeString("rel", []byte("nofollow noopener noreferrer")) | |
| 202 | + | a.SetAttributeString("rel", p.rel()) | |
| 203 | + | continue | |
| 204 | + | } | |
| 205 | + | if p.untrusted && a.AutoLinkType == ast.AutoLinkEmail && validMailbox(string(a.Label(p.source))) { | |
| 206 | + | a.SetAttributeString("rel", p.rel()) | |
| 173 | 207 | continue | |
| 174 | 208 | } | |
| 175 | 209 | // An email address or another scheme: the label as plain text. | |
| @@ -209,6 +243,9 @@ func (p linkPolicy) target(dest string) (string, linkKind) { | |||
| 209 | 243 | if dest == "" || strings.ContainsAny(dest, "\x00\r\n\t\\") { | |
| 210 | 244 | return "", dropped | |
| 211 | 245 | } | |
| 246 | + | if p.untrusted { | |
| 247 | + | return untrustedTarget(dest) | |
| 248 | + | } | |
| 212 | 249 | if strings.HasPrefix(dest, "#") { | |
| 213 | 250 | frag := strings.ToLower(strings.TrimPrefix(dest, "#")) | |
| 214 | 251 | if frag == "" || strings.ContainsAny(frag, " \"'<>") { | |
| @@ -256,6 +293,39 @@ func (p linkPolicy) target(dest string) (string, linkKind) { | |||
| 256 | 293 | return href, internal | |
| 257 | 294 | } | |
| 258 | 295 | ||
| 296 | + | // untrustedTarget keeps absolute http and https links with a host, and mailto | |
| 297 | + | // links to one plain address. | |
| 298 | + | func untrustedTarget(dest string) (string, linkKind) { | |
| 299 | + | if strings.HasPrefix(dest, "//") { | |
| 300 | + | return "", dropped | |
| 301 | + | } | |
| 302 | + | u, err := url.Parse(dest) | |
| 303 | + | if err != nil { | |
| 304 | + | return "", dropped | |
| 305 | + | } | |
| 306 | + | switch strings.ToLower(u.Scheme) { | |
| 307 | + | case "http", "https": | |
| 308 | + | if u.Host == "" || u.User != nil { | |
| 309 | + | return "", dropped | |
| 310 | + | } | |
| 311 | + | return u.String(), external | |
| 312 | + | case "mailto": | |
| 313 | + | if u.Opaque == "" || u.RawQuery != "" || !validMailbox(u.Opaque) { | |
| 314 | + | return "", dropped | |
| 315 | + | } | |
| 316 | + | return "mailto:" + u.Opaque, external | |
| 317 | + | } | |
| 318 | + | return "", dropped | |
| 319 | + | } | |
| 320 | + | ||
| 321 | + | // validMailbox tells whether s is a bare address: no space, quote, angle | |
| 322 | + | // bracket, percent sign, comma or second at sign. | |
| 323 | + | func validMailbox(s string) bool { | |
| 324 | + | local, host, ok := strings.Cut(s, "@") | |
| 325 | + | return ok && local != "" && host != "" && len(s) <= 254 && | |
| 326 | + | !strings.ContainsAny(local+host, " \t\r\n\"'<>%,;?#\\@") | |
| 327 | + | } | |
| 328 | + | ||
| 259 | 329 | func isWebURL(s string) bool { | |
| 260 | 330 | u, err := url.Parse(s) | |
| 261 | 331 | if err != nil || u.Host == "" { | |
| @@ -62,9 +62,25 @@ func (m *markdown) render(source []byte, repo, ref, dir string) (template.HTML, | |||||
| 62 | // #fragment links chosen, so that several documents on one page (the summaries | 62 | // #fragment links chosen, so that several documents on one page (the summaries | ||
| 63 | // of a run) do not share ids. | 63 | // of a run) do not share ids. | ||
| 64 | func (m *markdown) renderWithIDs(source []byte, repo, ref, dir, idPrefix string) (template.HTML, []Heading, error) { | 64 | func (m *markdown) renderWithIDs(source []byte, repo, ref, dir, idPrefix string) (template.HTML, []Heading, error) { | ||
| 65 | - | pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: idPrefix, inner: parser.NewContext().IDs()})) | 65 | + | return m.renderPolicy(source, linkPolicy{repo: repo, ref: ref, dir: dir, idPrefix: idPrefix}) |
| 66 | + | } | |||
| 67 | + | ||||
| 68 | + | // renderUntrusted renders Markdown written by an anonymous visitor (an issue | |||
| 69 | + | // approved from the public form). On top of the policy of render, which already | |||
| 70 | + | // drops raw HTML and never loads an image: | |||
| 71 | + | // - only http, https and mailto links are kept, every one with | |||
| 72 | + | // rel="nofollow noopener noreferrer ugc"; relative links, #fragments and | |||
| 73 | + | // every other scheme lose their link and keep the text; | |||
| 74 | + | // - nothing points into the repository. | |||
| 75 | + | func (m *markdown) renderUntrusted(source []byte, idPrefix string) (template.HTML, error) { | |||
| 76 | + | h, _, err := m.renderPolicy(source, linkPolicy{idPrefix: idPrefix, untrusted: true}) | |||
| 77 | + | return h, err | |||
| 78 | + | } | |||
| 79 | + | ||||
| 80 | + | func (m *markdown) renderPolicy(source []byte, pol linkPolicy) (template.HTML, []Heading, error) { | |||
| 81 | + | pc := parser.NewContext(parser.WithIDs(&prefixedIDs{prefix: pol.idPrefix, inner: parser.NewContext().IDs()})) | |||
| 66 | doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc)) | 82 | doc := m.md.Parser().Parse(text.NewReader(source), parser.WithContext(pc)) | ||
| 67 | - | pol := linkPolicy{repo: repo, ref: ref, dir: dir, source: source, idPrefix: idPrefix} | 83 | + | pol.source = source |
| 68 | pol.apply(doc) | 84 | pol.apply(doc) | ||
| 69 | 85 | ||||
| 70 | var buf bytes.Buffer | 86 | var buf bytes.Buffer | ||
| @@ -131,6 +147,20 @@ type linkPolicy struct { | |||||
| 131 | source []byte | 147 | source []byte | ||
| 132 | // idPrefix is the prefix of the heading ids that #fragment links point at. | 148 | // idPrefix is the prefix of the heading ids that #fragment links point at. | ||
| 133 | idPrefix string | 149 | idPrefix string | ||
| 150 | + | // untrusted is the policy of text from an anonymous visitor. | |||
| 151 | + | untrusted bool | |||
| 152 | + | } | |||
| 153 | + | ||||
| 154 | + | const ( | |||
| 155 | + | relExternal = "nofollow noopener noreferrer" | |||
| 156 | + | relUntrusted = "nofollow noopener noreferrer ugc" | |||
| 157 | + | ) | |||
| 158 | + | ||||
| 159 | + | func (p linkPolicy) rel() []byte { | |||
| 160 | + | if p.untrusted { | |||
| 161 | + | return []byte(relUntrusted) | |||
| 162 | + | } | |||
| 163 | + | return []byte(relExternal) | |||
| 134 | } | 164 | } | ||
| 135 | 165 | ||||
| 136 | func (p linkPolicy) apply(doc ast.Node) { | 166 | func (p linkPolicy) apply(doc ast.Node) { | ||
| @@ -163,13 +193,17 @@ func (p linkPolicy) apply(doc ast.Node) { | |||||
| 163 | l.Destination = []byte(dest) | 193 | l.Destination = []byte(dest) | ||
| 164 | l.Title = nil | 194 | l.Title = nil | ||
| 165 | if kind == external { | 195 | if kind == external { | ||
| 166 | - | l.SetAttributeString("rel", []byte("nofollow noopener noreferrer")) | 196 | + | l.SetAttributeString("rel", p.rel()) |
| 167 | } | 197 | } | ||
| 168 | } | 198 | } | ||
| 169 | for _, n := range autos { | 199 | for _, n := range autos { | ||
| 170 | a := n.(*ast.AutoLink) | 200 | a := n.(*ast.AutoLink) | ||
| 171 | if a.AutoLinkType == ast.AutoLinkURL && isWebURL(string(a.URL(p.source))) { | 201 | if a.AutoLinkType == ast.AutoLinkURL && isWebURL(string(a.URL(p.source))) { | ||
| 172 | - | a.SetAttributeString("rel", []byte("nofollow noopener noreferrer")) | 202 | + | a.SetAttributeString("rel", p.rel()) |
| 203 | + | continue | |||
| 204 | + | } | |||
| 205 | + | if p.untrusted && a.AutoLinkType == ast.AutoLinkEmail && validMailbox(string(a.Label(p.source))) { | |||
| 206 | + | a.SetAttributeString("rel", p.rel()) | |||
| 173 | continue | 207 | continue | ||
| 174 | } | 208 | } | ||
| 175 | // An email address or another scheme: the label as plain text. | 209 | // An email address or another scheme: the label as plain text. | ||
| @@ -209,6 +243,9 @@ func (p linkPolicy) target(dest string) (string, linkKind) { | |||||
| 209 | if dest == "" || strings.ContainsAny(dest, "\x00\r\n\t\\") { | 243 | if dest == "" || strings.ContainsAny(dest, "\x00\r\n\t\\") { | ||
| 210 | return "", dropped | 244 | return "", dropped | ||
| 211 | } | 245 | } | ||
| 246 | + | if p.untrusted { | |||
| 247 | + | return untrustedTarget(dest) | |||
| 248 | + | } | |||
| 212 | if strings.HasPrefix(dest, "#") { | 249 | if strings.HasPrefix(dest, "#") { | ||
| 213 | frag := strings.ToLower(strings.TrimPrefix(dest, "#")) | 250 | frag := strings.ToLower(strings.TrimPrefix(dest, "#")) | ||
| 214 | if frag == "" || strings.ContainsAny(frag, " \"'<>") { | 251 | if frag == "" || strings.ContainsAny(frag, " \"'<>") { | ||
| @@ -256,6 +293,39 @@ func (p linkPolicy) target(dest string) (string, linkKind) { | |||||
| 256 | return href, internal | 293 | return href, internal | ||
| 257 | } | 294 | } | ||
| 258 | 295 | ||||
| 296 | + | // untrustedTarget keeps absolute http and https links with a host, and mailto | |||
| 297 | + | // links to one plain address. | |||
| 298 | + | func untrustedTarget(dest string) (string, linkKind) { | |||
| 299 | + | if strings.HasPrefix(dest, "//") { | |||
| 300 | + | return "", dropped | |||
| 301 | + | } | |||
| 302 | + | u, err := url.Parse(dest) | |||
| 303 | + | if err != nil { | |||
| 304 | + | return "", dropped | |||
| 305 | + | } | |||
| 306 | + | switch strings.ToLower(u.Scheme) { | |||
| 307 | + | case "http", "https": | |||
| 308 | + | if u.Host == "" || u.User != nil { | |||
| 309 | + | return "", dropped | |||
| 310 | + | } | |||
| 311 | + | return u.String(), external | |||
| 312 | + | case "mailto": | |||
| 313 | + | if u.Opaque == "" || u.RawQuery != "" || !validMailbox(u.Opaque) { | |||
| 314 | + | return "", dropped | |||
| 315 | + | } | |||
| 316 | + | return "mailto:" + u.Opaque, external | |||
| 317 | + | } | |||
| 318 | + | return "", dropped | |||
| 319 | + | } | |||
| 320 | + | ||||
| 321 | + | // validMailbox tells whether s is a bare address: no space, quote, angle | |||
| 322 | + | // bracket, percent sign, comma or second at sign. | |||
| 323 | + | func validMailbox(s string) bool { | |||
| 324 | + | local, host, ok := strings.Cut(s, "@") | |||
| 325 | + | return ok && local != "" && host != "" && len(s) <= 254 && | |||
| 326 | + | !strings.ContainsAny(local+host, " \t\r\n\"'<>%,;?#\\@") | |||
| 327 | + | } | |||
| 328 | + | ||||
| 259 | func isWebURL(s string) bool { | 329 | func isWebURL(s string) bool { | ||
| 260 | u, err := url.Parse(s) | 330 | u, err := url.Parse(s) | ||
| 261 | if err != nil || u.Host == "" { | 331 | if err != nil || u.Host == "" { | ||
M internal/adapters/web/markdown_test.go +67 -0
| @@ -136,3 +136,70 @@ func TestMarkdownGFM(t *testing.T) { | |||
| 136 | 136 | assert.Contains(t, out, "<del>gone</del>") | |
| 137 | 137 | assert.Contains(t, out, `type="checkbox"`) | |
| 138 | 138 | } | |
| 139 | + | ||
| 140 | + | func renderUntrustedMD(t *testing.T, src string) string { | |
| 141 | + | t.Helper() | |
| 142 | + | h, err := newMarkdown().renderUntrusted([]byte(src), "issue-desc-") | |
| 143 | + | require.NoError(t, err) | |
| 144 | + | return string(h) | |
| 145 | + | } | |
| 146 | + | ||
| 147 | + | // An issue from the public form is rendered with the strictest policy. | |
| 148 | + | func TestMarkdownUntrusted(t *testing.T) { | |
| 149 | + | t.Parallel() | |
| 150 | + | tests := []struct { | |
| 151 | + | name, src string | |
| 152 | + | absent []string | |
| 153 | + | present []string | |
| 154 | + | }{ | |
| 155 | + | {"script", "<script>alert(1)</script>\n", []string{"<script", "alert(1)</"}, nil}, | |
| 156 | + | {"img onerror", "a <img src=x onerror=alert(1)> b\n", []string{"<img", "onerror"}, []string{"<p>a "}}, | |
| 157 | + | {"javascript link", "[x](javascript:alert(1))\n", []string{"javascript", "href"}, []string{"<p>x</p>"}}, | |
| 158 | + | {"data link", "[x](data:text/html,<script>alert(1)</script>)\n", []string{"data:", "href", "<script"}, nil}, | |
| 159 | + | {"vbscript and file", "[a](vbscript:x) [b](file:///etc/passwd)\n", []string{"vbscript", "file:", "href"}, []string{"a b"}}, | |
| 160 | + | {"relative links lose the link", "[a](../../etc) [b](/proj/blob/main/x) [c](#frag)\n", []string{"href"}, []string{"a b c"}}, | |
| 161 | + | {"protocol relative", "[x](//evil.test/p)\n", []string{"href", "evil"}, nil}, | |
| 162 | + | {"https is kept with ugc", "[x](https://example.org/p?q=1)\n", nil, | |
| 163 | + | []string{`href="https://example.org/p?q=1"`, `rel="nofollow noopener noreferrer ugc"`}}, | |
| 164 | + | {"http is kept", "[x](http://example.org/)\n", nil, []string{`href="http://example.org/"`, "ugc"}}, | |
| 165 | + | {"credentials are refused", "[x](https://user:pw@example.org/)\n", []string{"href", "pw"}, nil}, | |
| 166 | + | {"mailto is kept", "[x](mailto:a@example.org)\n", nil, []string{`href="mailto:a@example.org"`, "ugc"}}, | |
| 167 | + | {"mailto with query is refused", "[x](mailto:a@example.org?cc=b@example.org)\n", []string{"href"}, nil}, | |
| 168 | + | {"autolink url", "<https://example.org/a> and <javascript:alert(1)>\n", []string{"href=\"javascript"}, []string{`rel="nofollow noopener noreferrer ugc"`}}, | |
| 169 | + | {"autolink email", "<a@example.org>\n", nil, []string{`href="mailto:a@example.org"`, "ugc"}}, | |
| 170 | + | {"bare url (GFM)", "see https://example.org/x now\n", nil, []string{`href="https://example.org/x"`, "ugc"}}, | |
| 171 | + | {"image is its alt text", "\n", []string{"<img", "evil.test"}, []string{`<span class="alt">[pwn]</span>`}}, | |
| 172 | + | {"reference link to javascript", "[x][r]\n\n[r]: javascript:alert(1)\n", []string{"javascript", "href"}, nil}, | |
| 173 | + | {"entity-encoded scheme", "[x](javascript:alert(1))\n", []string{"avascript:", "href"}, nil}, | |
| 174 | + | } | |
| 175 | + | for _, tt := range tests { | |
| 176 | + | t.Run(tt.name, func(t *testing.T) { | |
| 177 | + | t.Parallel() | |
| 178 | + | out := renderUntrustedMD(t, tt.src) | |
| 179 | + | for _, a := range tt.absent { | |
| 180 | + | assert.NotContains(t, out, a) | |
| 181 | + | } | |
| 182 | + | for _, p := range tt.present { | |
| 183 | + | assert.Contains(t, out, p) | |
| 184 | + | } | |
| 185 | + | }) | |
| 186 | + | } | |
| 187 | + | } | |
| 188 | + | ||
| 189 | + | func TestMarkdownUntrustedBigInputs(t *testing.T) { | |
| 190 | + | t.Parallel() | |
| 191 | + | long := strings.Repeat("a", 10000) | |
| 192 | + | out := renderUntrustedMD(t, long) | |
| 193 | + | assert.Contains(t, out, long) | |
| 194 | + | ||
| 195 | + | rows := "| a | b |\n|---|---|\n" + strings.Repeat("| <b>x</b> | [l](javascript:1) |\n", 500) | |
| 196 | + | out = renderUntrustedMD(t, rows) | |
| 197 | + | assert.NotContains(t, out, "<b>") | |
| 198 | + | assert.NotContains(t, out, "javascript") | |
| 199 | + | assert.NotContains(t, out, "style=") | |
| 200 | + | ||
| 201 | + | deep := strings.Repeat("> ", 200) + "[x](javascript:1)\n" | |
| 202 | + | assert.NotContains(t, renderUntrustedMD(t, deep), "javascript") | |
| 203 | + | ||
| 204 | + | assert.NotContains(t, renderUntrustedMD(t, "[x](https://a.test/\"onmouseover=\"alert(1))\n"), "onmouseover=\"") | |
| 205 | + | } | |
| @@ -136,3 +136,70 @@ func TestMarkdownGFM(t *testing.T) { | |||||
| 136 | assert.Contains(t, out, "<del>gone</del>") | 136 | assert.Contains(t, out, "<del>gone</del>") | ||
| 137 | assert.Contains(t, out, `type="checkbox"`) | 137 | assert.Contains(t, out, `type="checkbox"`) | ||
| 138 | } | 138 | } | ||
| 139 | + | ||||
| 140 | + | func renderUntrustedMD(t *testing.T, src string) string { | |||
| 141 | + | t.Helper() | |||
| 142 | + | h, err := newMarkdown().renderUntrusted([]byte(src), "issue-desc-") | |||
| 143 | + | require.NoError(t, err) | |||
| 144 | + | return string(h) | |||
| 145 | + | } | |||
| 146 | + | ||||
| 147 | + | // An issue from the public form is rendered with the strictest policy. | |||
| 148 | + | func TestMarkdownUntrusted(t *testing.T) { | |||
| 149 | + | t.Parallel() | |||
| 150 | + | tests := []struct { | |||
| 151 | + | name, src string | |||
| 152 | + | absent []string | |||
| 153 | + | present []string | |||
| 154 | + | }{ | |||
| 155 | + | {"script", "<script>alert(1)</script>\n", []string{"<script", "alert(1)</"}, nil}, | |||
| 156 | + | {"img onerror", "a <img src=x onerror=alert(1)> b\n", []string{"<img", "onerror"}, []string{"<p>a "}}, | |||
| 157 | + | {"javascript link", "[x](javascript:alert(1))\n", []string{"javascript", "href"}, []string{"<p>x</p>"}}, | |||
| 158 | + | {"data link", "[x](data:text/html,<script>alert(1)</script>)\n", []string{"data:", "href", "<script"}, nil}, | |||
| 159 | + | {"vbscript and file", "[a](vbscript:x) [b](file:///etc/passwd)\n", []string{"vbscript", "file:", "href"}, []string{"a b"}}, | |||
| 160 | + | {"relative links lose the link", "[a](../../etc) [b](/proj/blob/main/x) [c](#frag)\n", []string{"href"}, []string{"a b c"}}, | |||
| 161 | + | {"protocol relative", "[x](//evil.test/p)\n", []string{"href", "evil"}, nil}, | |||
| 162 | + | {"https is kept with ugc", "[x](https://example.org/p?q=1)\n", nil, | |||
| 163 | + | []string{`href="https://example.org/p?q=1"`, `rel="nofollow noopener noreferrer ugc"`}}, | |||
| 164 | + | {"http is kept", "[x](http://example.org/)\n", nil, []string{`href="http://example.org/"`, "ugc"}}, | |||
| 165 | + | {"credentials are refused", "[x](https://user:pw@example.org/)\n", []string{"href", "pw"}, nil}, | |||
| 166 | + | {"mailto is kept", "[x](mailto:a@example.org)\n", nil, []string{`href="mailto:a@example.org"`, "ugc"}}, | |||
| 167 | + | {"mailto with query is refused", "[x](mailto:a@example.org?cc=b@example.org)\n", []string{"href"}, nil}, | |||
| 168 | + | {"autolink url", "<https://example.org/a> and <javascript:alert(1)>\n", []string{"href=\"javascript"}, []string{`rel="nofollow noopener noreferrer ugc"`}}, | |||
| 169 | + | {"autolink email", "<a@example.org>\n", nil, []string{`href="mailto:a@example.org"`, "ugc"}}, | |||
| 170 | + | {"bare url (GFM)", "see https://example.org/x now\n", nil, []string{`href="https://example.org/x"`, "ugc"}}, | |||
| 171 | + | {"image is its alt text", "\n", []string{"<img", "evil.test"}, []string{`<span class="alt">[pwn]</span>`}}, | |||
| 172 | + | {"reference link to javascript", "[x][r]\n\n[r]: javascript:alert(1)\n", []string{"javascript", "href"}, nil}, | |||
| 173 | + | {"entity-encoded scheme", "[x](javascript:alert(1))\n", []string{"avascript:", "href"}, nil}, | |||
| 174 | + | } | |||
| 175 | + | for _, tt := range tests { | |||
| 176 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 177 | + | t.Parallel() | |||
| 178 | + | out := renderUntrustedMD(t, tt.src) | |||
| 179 | + | for _, a := range tt.absent { | |||
| 180 | + | assert.NotContains(t, out, a) | |||
| 181 | + | } | |||
| 182 | + | for _, p := range tt.present { | |||
| 183 | + | assert.Contains(t, out, p) | |||
| 184 | + | } | |||
| 185 | + | }) | |||
| 186 | + | } | |||
| 187 | + | } | |||
| 188 | + | ||||
| 189 | + | func TestMarkdownUntrustedBigInputs(t *testing.T) { | |||
| 190 | + | t.Parallel() | |||
| 191 | + | long := strings.Repeat("a", 10000) | |||
| 192 | + | out := renderUntrustedMD(t, long) | |||
| 193 | + | assert.Contains(t, out, long) | |||
| 194 | + | ||||
| 195 | + | rows := "| a | b |\n|---|---|\n" + strings.Repeat("| <b>x</b> | [l](javascript:1) |\n", 500) | |||
| 196 | + | out = renderUntrustedMD(t, rows) | |||
| 197 | + | assert.NotContains(t, out, "<b>") | |||
| 198 | + | assert.NotContains(t, out, "javascript") | |||
| 199 | + | assert.NotContains(t, out, "style=") | |||
| 200 | + | ||||
| 201 | + | deep := strings.Repeat("> ", 200) + "[x](javascript:1)\n" | |||
| 202 | + | assert.NotContains(t, renderUntrustedMD(t, deep), "javascript") | |||
| 203 | + | ||||
| 204 | + | assert.NotContains(t, renderUntrustedMD(t, "[x](https://a.test/\"onmouseover=\"alert(1))\n"), "onmouseover=\"") | |||
| 205 | + | } | |||