rustdoc: parenthetical after attribute syntax breaks HTML

[kmcallister]

/// Declare with
/// #[foo] (my favorite attribute)
#[foo]
pub fn main() { }

produces

<pre class='rust fn'>pub fn main()</pre><div class='docblock'><p>Declare with</p>

<h1 id="<a-href="my%20favorite%20attribute">foo</a>" class='section-header'><a
                           href="#<a-href="my%20favorite%20attribute">foo</a>"><a href="my%20favorite%20attribute">foo</a></a></h1>

because it parses as a Markdown link. The right thing to do is put backtics around the attribute syntax, but this is still a really confusing failure.

It would also be an XSS vulnerability if we hosted docs generated from user contributed libraries:

/// Declare with
/// #[foo] (my favorite attribute) <script>alert(document.cookie)</script>

But the conservative solution there is to put user content on another origin (e.g. rust-user-content.org) which doesn't hold any credentials.

[huonw]

HTML in markdown get interpreted literally i.e.

/// <script>alert("hi");</script>
fn foo() {}

is also an XSS risk:

<div class='docblock'><script>alert("hi");</script>
</div></section>

[alexcrichton]

The first one I view as a fact of using markdown, and can be fixed with:

/// Declare with
/// \#[foo] \(my favorite attribute)
#[foo]
pub fn main() { }

The second one I view as bit of not a bug because you're the one writing documentation, so it's not necessarily XSS because no one is injecting anything into your page?

[huonw]

If we had a public service hosting documentation (well, anyone hosting such things, I guess Rust CI may be affected), then someone can write a Rust library that injects javascript into that site, meaning it should be hosted on something with no credentials.

[tomjakubowski]

It would probably be reasonable to add an option to rustdoc which enables Hoedown's HTML_ESCAPE or HTML_SKIP for this case of hosting rustdocs on a site with some kind of credentials in use, or perhaps one that uses a post-processor to remove <script> tags and javascript: links and event attributes (onclick et al.) from the generated HTML.

[kmcallister]

a post-processor to remove <script> tags and javascript: links and event attributes (onclick et al.) from the generated HTML.

Doing this safely requires completely parsing the HTML and whitelisting tags / attributes at the AST level, then re-serializing as well-formed HTML. Anything short of that will have loopholes due to browsers' attempts to interpret seriously malformed HTML (see The Tangled Web chapter 4).

[alexcrichton]

I'm going to close this as not-a-bug as by using Markdown we're committing ourselves to one form of syntax or another, and any XSS vulnerabilities or vectors would probably be handled when we set up hosting!

[tomjakubowski]

I'd recommend that we make a very clear warning somewhere that hosting docs for arbitrary crates opens up XSS attacks and proper precautions should be taken. It's important to remember that crates.io isn't the only domain hosting rustdocs.

—
Sent from Mailbox

On Mon, Apr 6, 2015 at 3:18 PM, Alex Crichton notifications@github.com
wrote:

I'm going to close this as not-a-bug as by using Markdown we're committing ourselves to one form of syntax or another, and any XSS vulnerabilities or vectors would probably be handled when we set up hosting!

Reply to this email directly or view it on GitHub:
#14814 (comment)

[tomjakubowski]

Er, I mean docs.rust-lang.org or whatever. Although it's conceivable that crates.io (or a sub domain) will host docs as well some day.

—
Sent from Mailbox

On Mon, Apr 6, 2015 at 3:18 PM, Alex Crichton notifications@github.com
wrote:

Closed #14814.

Reply to this email directly or view it on GitHub:
#14814 (comment)

[kmcallister]

Better register crates-user-content.io...

[kmcallister]

Doing this safely requires completely parsing the HTML and whitelisting tags / attributes at the AST level, then re-serializing as well-formed HTML.

fwiw I think html5ever is up to this task now.

[alexcrichton]

@tomjakubowski good idea! I've opened #24160

[lambda-fairy]

@kmcallister According to rust-lang/crates.io#91, some members of the community have reserved rustdoc.org and doc.rs for this purpose. So getting a domain is not a blocker.