Skip to content
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

Improve documentation for supplying valid escape/sanitize functions #205

Open
Tracked by #231
Abdullilah opened this issue May 24, 2022 · 7 comments
Open
Tracked by #231

Comments

@Abdullilah
Copy link

It would be good if this plugin excluded the code which is sanitised by the sanitize function from the DomSanitizer.

Example:
this.hostElement.innerHTML = content;

this is unsanitized content which makes sense for the plugin to complain about it, but when we sanitise the content:

this.hostElement.innerHTML = <string>this.domSanitizer.sanitize(SecurityContext.HTML, content);

I am still getting Eslint unsafe assignment even though the content I am adding is fully sanitised.

Could you please have a look and add this improvement to the plugin?

@mozfreddyb
Copy link
Collaborator

We allow custom sanitizers through configurations.
See this testcase to check that users can allow DOMPurify:

{ // issue 108: adding tests for custom escaper
code: "w.innerHTML = DOMPurify.sanitize('<em>${evil}</em>');",
parserOptions: { ecmaVersion: 6 },
options: [
{
escape: {
methods: ["DOMPurify.sanitize"]
}
}
]
},
(Added in #108).

Does that not work for you?

@Abdullilah
Copy link
Author

@mozfreddyb Thanks for your comment.

Could you please tell me how to add it exactly to the configuration?

I tried to add this line to the eslint rules:

"extends": ["plugin:no-unsanitized/DOM"],
"rules": {"no-unsanitized/method": ["error", { "escape": { "methods": ["DomSanitizer.sanitize"] } }]}

OR

"extends": ["plugin:no-unsanitized/DOM"],
"rules": {"no-unsanitized/method": ["error", { "escape": { "methods": ["DOMPurify.sanitize"] } }]}

but I am still getting the same eslint error:

Screen Shot 2022-06-01 at 12 25 34

@mozfreddyb
Copy link
Collaborator

In your example, you're modifying the options of the no-unsanitized/method, which protects you from calling methods (e.g., insertAdjacentHTML(), document.write(), ..).

You also need to do this for the no-unsanitized/property rule which protects properties (e.g., .innerHTML)

@mozfreddyb
Copy link
Collaborator

@Abdullilah Did you end up resolving your issue? I'm leaning towards closing this issue.

@rszczypka
Copy link

This works but only when done this way

"rules": {
"no-unsanitized/method": ["error", { "escape": { "methods": ["this.domSanitizer.sanitize"] } }],
"no-unsanitized/property": ["error", { "escape": { "methods": ["this.domSanitizer.sanitize"] } }],
}

@mozfreddyb
Copy link
Collaborator

Looks like everything works as intended here. We can repurpose the issue, if someone wants to update the documentation though.

@mozfreddyb mozfreddyb changed the title Exclude the sanitised code with DomSanitizer sanitize function Improve documentation for supplying valid escape/sanitize functions Nov 15, 2023
@mozfreddyb
Copy link
Collaborator

I think this could be more easily tested with the integration tests that @rpl is introducing in #244

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants