Skip to content

rustdoc: escape shown input to prevent injection #13895

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 4, 2014

Conversation

adrientetar
Copy link
Contributor

See #13884 for the details. Closes #13884.

r? @alexcrichton

@alexcrichton
Copy link
Member

Since we have jquery, can we rely on jquery instead?

@adrientetar
Copy link
Contributor Author

It seems hacky to use in this context because those are getters methods (have to be applied to a DOM element?).

This page linked on the stackoverflow post suggests:

$('<div/>').text('This is fun & stuff').html(); // evaluates to "This is fun &amp; stuff"

...with a fake DOM element as a shim. The last comment on the page even states:

Be careful using this. Whenever you use jQuery's function .html(val)
with val coming from user input, you will be introducing xss vulnerability as it will be evaluated if there are any script tags.

@alexcrichton
Copy link
Member

I think that comment may be a red herring referring to some other method of adding text to a page.

I think something like this should do the trick:

output = $('<h1/>').text('Results for ' + query.query + ...).html()
output += ...;

@alexcrichton
Copy link
Member

As in, I think that the comment is generally referring to executing html(val), not executing html(val) where val has already been escaped.

My concern from defining our own escape function stems from the fact that this is semi-security-related, and in things dealing with security it's generally a bad idea to "roll your own". In this case, I trust jquery to get escaping right, so relying on them should be a bit more robust than relying on ourselves.

@steveklabnik
Copy link
Member

Yeah, +1 on that. I'm sure jQuery's escaping is awesome.

@adrientetar
Copy link
Contributor Author

The $('<h1/>') part in your example is a shim because .text() is a method, it doesn't actually do anything (which is why I don't like this pattern); you still need "<h1>" and so on inside.

Anyways, updated.

bors added a commit that referenced this pull request May 4, 2014
@bors bors closed this May 4, 2014
@bors bors merged commit 5f0a426 into rust-lang:master May 4, 2014
bors pushed a commit to rust-lang-ci/rust that referenced this pull request Jan 28, 2025
I opened rust-lang/rust-clippy#13896 before.
However, I found that there're more cases where Clippy suggests to use
modules that belong to the `std` crate even in a `no_std` environment.
Therefore, this PR include the changes I've made in rust-lang#13896 and new
changes to fix cases I found this time to prevent wrong suggestions in
`no_std` environments as well.

changelog: [`redundant_closure`]: correct suggestion in `no_std`
changelog: [`repeat_vec_with_capacity`]: correct suggestion in `no_std`
changelog: [`single_range_in_vec_init`]: don't emit suggestion to use
`Vec` in `no_std`
changelog: [`drain_collect`]: correct suggestion in `no_std`
changelog: [`map_with_unused_argument_over_ranges`]: correct suggestion
in `no_std`

also close rust-lang#13895
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rustdoc search xss exploit
4 participants