Shlok Parida

Your Markdown renderer blocks four schemes

goldmark refuses a link destination beginning with javascript:, vbscript:, file: or data: — and then allows data:image/png;, gif;, jpeg; and webp; back through. A denylist with a small allowlist cut into it. It is a list of the schemes somebody thought of, which is a different thing from a list of the schemes that are safe.

Stock goldmark v1.8.5, CommonMark plus GFM, WithUnsafe not set:

[click](javascript:alert(1))          <a href="">click</a>
[click](JaVaScRiPt:alert(1))          <a href="">click</a>
[click](  javascript:alert(1))        <a href="">click</a>
[click](data:text/html,<script>…)     <a href="">click</a>
<script>alert(1)</script>             <!-- raw HTML omitted -->

Those are handled properly — case folded, leading whitespace trimmed, autolinks included. That part is not the problem, and the common blog post claiming a case-sensitivity bypass here is wrong. Note the carve-out applies to any destination, not only to images, so [click](data:image/png;base64,…) renders as a link; data:image/svg+xml stays blocked, which is the one you would have worried about.

The problem is everything not on the list

Same renderer, same settings:

[click](//evil.example.com/phish)     <a href="//evil.example.com/phish">click</a>
[click](steam://run/730)              <a href="steam://run/730">click</a>
[click](intent://scan/#Intent;end)    <a href="intent://scan/#Intent;end">click</a>

The first one is the one that matters, and not only because goldmark permits it. It also survives the check you would write yourself:

strings.HasPrefix(dest, "/")     // true for //evil.example.com

A browser reads //host/path as protocol-relative — an absolute URL to another origin, borrowing the current page's scheme. So a rule that you believe means "site-relative links are fine" in fact admits every host on the internet, and the link renders looking local right up until someone clicks it.

The custom schemes are a smaller problem that never gets smaller. intent:// on Android, and a long tail of handlers registered by desktop applications, turn a destination in user-submitted Markdown into a message to a local program.

Invert it

The replacement is shorter than the thing it replaces, which is usually the sign that a denylist was the wrong shape:

func safeURL(u string) (string, bool) {
	u = strings.TrimSpace(u)
	if strings.ContainsRune(u, '\\') || strings.IndexFunc(u, isURLControl) >= 0 {
		return "", false
	}
	lower := strings.ToLower(u)
	switch {
	case strings.HasPrefix(lower, "http://"),
		strings.HasPrefix(lower, "https://"),
		strings.HasPrefix(lower, "mailto:"):
		return u, true
	case strings.HasPrefix(u, "//"):   // protocol-relative, not site-relative
		return "", false
	case strings.HasPrefix(u, "/"), strings.HasPrefix(u, "#"):
		return u, true
	}
	return "", false
}

func isURLControl(r rune) bool { return r < 0x20 || r == 0x7f }

The // case has to be tested before the / case. In the other order it is unreachable and the bug outlives the fix that was supposed to remove it.

The prefix check is not enough by itself

I shipped that function without the first three lines, and it was wrong. A browser rewrites a URL before it parses it, and two of those rewrites turn something that reads as site-relative into another origin.

Resolved against https://example.com/posts/x by the WHATWG parser every browser implements:

/\evil.example.com/phish   →  https://evil.example.com/phish
/<tab>/evil.example.com    →  https://evil.example.com/

Under http and https a backslash is treated as a slash, so /\host is //host. Tab, carriage return and line feed are stripped from anywhere in the string, so /<tab>/host is also //host — and strings.TrimSpace only touches the ends, which is exactly the sort of function that looks like it handles whitespace.

Both pass strings.HasPrefix(u, "/"). Both fail the // test. Both leave your origin. Reject backslashes and control characters outright rather than trying to normalise them: a destination containing either is not a link anybody meant to write.

Three node types, and the third one is the one you forget

A destination reaches the output through ast.Link, ast.Image and ast.AutoLink. The first two are obvious. The third is not: <scheme:anything> is a valid CommonMark autolink for any scheme, so it needs the same check — and unlike a link it has no child text to fall back on, so it has to be replaced with a text node rather than unwrapped.

Unwrapping rather than deleting is the point. The reader keeps the words and loses only the navigation:

[click](steam://run/730)   →   <p>click</p>

A stripped link that takes the sentence with it is a denial-of-service on your own content, and it is what makes people turn sanitising off.

One goldmark-specific trap: an AST transformer that mutates nodes during ast.Walk invalidates the walk. Collect the offending nodes first, edit afterwards.

Write it down as a test

This is the rare security control that is trivial to test, because the input and the output are both strings:

for _, c := range []struct{ in, want string }{
	{"[a](//evil.example.com)", "<p>a</p>"},
	{"[a](/about)", `<p><a href="/about">a</a></p>`},
	{"<javascript:alert(1)>", "<p>javascript:alert(1)</p>"},
} { … }

Every scheme someone reports later becomes one more line. A denylist grows the same way, but each line is a hole that was open until somebody found it.

Enumerate what you support

A denylist enumerates the attacks known when it was written; an allowlist enumerates the uses you support, and that list is short, stable, and something you can actually hold in your head. Write down the three or four destinations your site is for, reject the rest, and test the protocol-relative case specifically — it is the one that looks like the safe branch.