| Github | https://github.com/lourencovales |
| Excipio | https://excipio.tech/ |
| Github | https://github.com/lourencovales |
| Excipio | https://excipio.tech/ |
Now, is this a _potential_ bug? Yes, if e.g. the goldmark package ever changes the way it processes its input and creates the possibility of the conversion erroring out, then it's possible for an attacker to craft a payload that could trigger this code path and exploit this XSS vulnerability.
And should the devs just have used `return template.HTML(escapedText)` in that code path? Also yes. Why they didn't, I'm not really sure. But it's besides the point.
The return inside the error checking is for the unescaped text, and ideally should use escapedText. But is it possible to even reach this situation? For that, the function utils.MarkdownToHTML() would need to error out.
And can it? No. Turns out the error checking is actually *dead code*. If you check how the goldmark package handles input, you could see that that it's not possible for this path to ever be reachable.
The problematic code is this:
```
func prepareTextForEmail(text, siteURL string) template.HTML {
escapedText := html.EscapeString(text)
markdownText, err := utils.MarkdownToHTML(escapedText, siteURL)
if err != nil {
mlog.Warn("Encountered error while converting markdown to HTML", mlog.Err(err))
return template.HTML(text)
}
return template.HTML(markdownText)
}
```