Return to Answer

Commonmark migration

Source Link

edited Jun 10, 2020 at 13:24

Community Bot

The Attack

What you are basically talking about is DOM based XSS.

But it's a self-XSS, so the only possible attack vector might be via ClickJacking (depending on the used browser).

Still, it should be defended against, especially for usability reasons.

Proper Defense

Here is OWASPs guide on preventing DOM based XSS. Basically, you should first HTML encode the input, and then JavaScript encode.

For your example, the second part doesn't seem necessary, just encoding <, >, ' and " should be enough.

Your Defense

Well, your defense works.

It could be simplified a bit though. There really doesn't seem to be a need to set this.textContent, and I also do not see the need for replace(/^\s+|\s+$/g, '') (or am I missing something here? Browser compatibility maybe?).

That would leave you with

var safetext = (function(text){
    this.textContent = text;
    return this.innerHTML;
}).bind(document.createElement('div'));

The reason this works seems to be the way textContent works.

But this really seems to be more of a side-effect, and it is never a good idea to use those for security related things.

Conclusion

Regarding performance, security, and readability, a simple replace for <, >, ' and " with their HTML representation seems to be preferable to your method.

The Attack

What you are basically talking about is DOM based XSS.

But it's a self-XSS, so the only possible attack vector might be via ClickJacking (depending on the used browser).

Still, it should be defended against, especially for usability reasons.

Proper Defense

Here is OWASPs guide on preventing DOM based XSS. Basically, you should first HTML encode the input, and then JavaScript encode.

For your example, the second part doesn't seem necessary, just encoding <, >, ' and " should be enough.

Your Defense

Well, your defense works.

That would leave you with

var safetext = (function(text){
    this.textContent = text;
    return this.innerHTML;
}).bind(document.createElement('div'));

The reason this works seems to be the way textContent works.

But this really seems to be more of a side-effect, and it is never a good idea to use those for security related things.

Conclusion

Regarding performance, security, and readability, a simple replace for <, >, ' and " with their HTML representation seems to be preferable to your method.

The Attack

What you are basically talking about is DOM based XSS.

But it's a self-XSS, so the only possible attack vector might be via ClickJacking (depending on the used browser).

Still, it should be defended against, especially for usability reasons.

Proper Defense

Here is OWASPs guide on preventing DOM based XSS. Basically, you should first HTML encode the input, and then JavaScript encode.

For your example, the second part doesn't seem necessary, just encoding <, >, ' and " should be enough.

Your Defense

Well, your defense works.

That would leave you with

var safetext = (function(text){
    this.textContent = text;
    return this.innerHTML;
}).bind(document.createElement('div'));

The reason this works seems to be the way textContent works.

But this really seems to be more of a side-effect, and it is never a good idea to use those for security related things.

Conclusion

Regarding performance, security, and readability, a simple replace for <, >, ' and " with their HTML representation seems to be preferable to your method.

Source Link

created Apr 14, 2016 at 12:56

tim

25.1k
3
30
76

The Attack

What you are basically talking about is DOM based XSS.

But it's a self-XSS, so the only possible attack vector might be via ClickJacking (depending on the used browser).

Still, it should be defended against, especially for usability reasons.

Proper Defense

Here is OWASPs guide on preventing DOM based XSS. Basically, you should first HTML encode the input, and then JavaScript encode.

For your example, the second part doesn't seem necessary, just encoding <, >, ' and " should be enough.

Your Defense

Well, your defense works.

That would leave you with

var safetext = (function(text){
    this.textContent = text;
    return this.innerHTML;
}).bind(document.createElement('div'));

The reason this works seems to be the way textContent works.

But this really seems to be more of a side-effect, and it is never a good idea to use those for security related things.

Conclusion

Regarding performance, security, and readability, a simple replace for <, >, ' and " with their HTML representation seems to be preferable to your method.