XSS vulnerability found

[Xaeroxe]

https://docs.rs/pwnies

While hilarious, this should probably be fixed.

[onur]

Thanks for reporting, and thank you @isislovecruft for cute ponies!

I've been aware of this issue since I made docs.rs available to public. This issue has been demonstrated in the xss-probe by @Diggsey in the very first day of docs.rs. And docs.rs didn't even support metadata section, so I still favor his way of doing.

But to me this isn't an issue at all, it is just a nice feature of rustdoc. This is allowing crate owners to use their preferred way to customize and extend their crate documentation. For example, when I first introduced rustdoc-args, people got excited about possible KaTeX usage in num crate. Too bad it didn't happen but @isislovecruft also discovered this when she tried to use KaTeX in her own crate curve25519-dalek. It wouldn't be possible without this feature.

Also docs.rs is not advocating any crate. Users have to explicitly enter name of the crate they are looking for. How many of you knew about xss-probe crate, did anyone actually see it? It is not any different than creating a GitHub page and doing funny stuff in it. Documentations are part of the crate not docs.rs. Docs.rs is just providing an easy way to access documentation of a crate you are looking for. And docs.rs is not using any kind of private information (actually docs.rs is not using any cookie at all), so there is actually nothing to steal with an XSS vulnerability.

If someone abuses this and hurts users in some way, we can basically blacklist owner of the crate. But TBH I don't see how it is possible unless some high profile crate owners (like hyper, iron etc.) decides to abuse their own users from their own crate documentation.

[Xaeroxe]

Current usage of it is pretty harmless. It could be used to track how users browse the documentation, which is also arguably a feature, or arguably an invasion of privacy.

However there is also the opportunity for more malicious and hidden usage. Some sites for example have started using JavaScript to mine cryptocurrency with the visitor's processing power.

The impact of this is honestly much lower than most XSS would be, but it is still possible this could be used maliciously. Arbitrary code execution is a real big can of worms.

[Xaeroxe]

As far as I'm concerned it's kind of a toss-up between bug or feature. You've raised some good points about having to specifically visit the crate's documentation, and the fact that this has many genuinely useful uses. There is a little bit of potential for exploitation, but not so much as to be overwhelmingly concerning. So I wouldn't be upset if this was closed, but I also wouldn't mind it being fixed.

[onur]

Let's assume that this becomes a really serious issue in the future and every rust crate author decided to mine cryptocurrency from documentation of their crates.

How do we fix it?

We can use CSP to block usage of external resources and docs.rs can use a camo server which is similar to GitHub to serve images from external sources but still, people can simply add malicious code into their own crate and copy it into the documentation at build time with their build script and use docs.rs to poison their own users.

I don't think it is possible to verify content of documentation.

[Xaeroxe]

It would require sanitization of the rendered html, similar to what GitHub does when rendering a README.md.

External resources aren't so much the issue as much as ones within the crate itself.

[onur]

So basically removing any script tag not generated by rustdoc should fix this issue, but TBH I'd like to hear everyone's opinion on this before making any decision. I still believe this is a nice feature of rustdoc and docs.rs should support this instead of blocking any script usage.

[GuillaumeGomez]

I like the fact that you can put script into docs, so if possible I'd like us to keep it. However, if any crate is abusing it, it should be blocked. Therefore, should a "report" be added?

[est31]

One can use the ammonia crate to sanitize html. This can happen as a step after rustdoc ran. From my own experience it is a bit too strict however, banning too much stuff. cc @notriddle

IMO scripts inside documentation should be handled on a whitelist based approach, with each crate that wants to use javascript having to first tell you what it intends to use javascript for, and only then getting the javascript stripping turned off.

[Havvy]

I like both @GuillaumeGomez and @est31's ideas. Have a report option that notifies you and the Rust moderation team and what a whitelist approach for crates with custom JS.

That said, make sure to whitelist the pwnies crate if you do make a whitelist. And then keep xss-probe blacklisted, just to make sure the blacklist works.

Also, since you asked, this was the first I've heard of xss-probe.

[freddyb]

👍 to using ammonia. It's too risky "just" to remove <script> tags.

[notriddle]

If you want to remove JS, then you want to use ammonia probably. But I don't think it matters if docs.rs removes JS. Supporting JS on here is a feature, not a big, and removing it would be a breaking change.

[freddyb]

hm, security-me got over-excited maybe.
Is it indeed a feature to allow everyone upload their scripts into the same origin? Maybe one could consider moving project docs into their own subdomain, to allow proper sharding. Happy to extend if someone's willing to listen to my ramblings :-)

[marksteward]

Don't forget you can install a serviceworker, which can then rewrite pages of other users.

[est31]

rustdoc's choice to use ember won't really improve the situation with rustdoc 2.0 but rather mean the reverse as now we can't just simply ban that pesky JavaScript.... Maybe docs.rs could use an alternative non dynamic AOT frontend?

[GuillaumeGomez]

@steveklabnik decided that new rustdoc would use ember so maybe ask him directly?