Scratchaddons: We will reject addons using innerHTML or triple curly from now on

Created on 2 Sep 2020  Â·  27Comments  Â·  Source: ScratchAddons/ScratchAddons

Please do not use innerHTML. Use appendChild or textContent or innerText. Existing addons will be changed to use these methods. This includes Vue {{{.

announcement security

Most helpful comment

image

All 27 comments

Kkkkkk I always use appendChild bc innerHTML doesn’t work for me sometimes. innerHTML is easier tho

image

This is gonna sound stupid, but… What is innerhtml?

This is gonna sound stupid, but… What is innerhtml?

in javascript there are many ways to interact with the html page. one of them is directly modifying the HTML which is bad because it opens up for attacks like XSS and stuff.

Oh, I see. Thanks! :)

The issue isn't innerHTML itself really (even tho it's dangerous if you don't know what you're doing), but the review process in extension stores. They don't like innerHTML.

I don't think you can say that all innerHTML implementation is bad. Sure that "innerHTML is bad because it parses it directly so they can XSS", but I don't think that's the case here.

But, sure, using textContent is feasable.

Maybe add an "announcement" label for this type of issues?

Pins are enough, perhaps?

Pins are enough, perhaps?

We only have limited numbers (to be more specific, 3) of pins. So accessing older ones is hard without an unique label.

Vue.js {{{ is now on the denylist. If you really need to use it, please ask me. The only allowed usage for now is for comments (because they are sanitized server side).

What is Vue {{{?

Works same as innerHTML.

what about innerText

innerText is safe.

K

Question, @apple502j: What about insertAdjacentHTML()?

@NoobTracker As long as you can add HTML, don't.

Will an addon with this get rejected?

element.innerHTML = string.replace(/</g,"&lt;").replace(/>/g,"&gt;");

@apple502j I think that it's used somewhere in an extension ... but don't ask me which one.

@apple502j I think that it's used somewhere in an extension ... but don't ask me which one.

Just search it in this repo and you will find.

Well I dunno how to do that.
I think @JeffaloBob did it ...

Will an addon with this get rejected?

element.innerHTML = string.replace(/</g,"&lt;").replace(/>/g,"&gt;");

Why not just use textContent? Easier to avoid XSS.

@easrng But what if you want, for example, this?

element.innerHTML = string.replace(/</g,"&lt;").replace(/>/g,"&gt;").replace(/\n/g,"<br>");

Well @BoomerScratch it seems like the reason why this isn't allowed anymore is that the addon people don't like it, although it might be used in completely harmless situations.

@easrng But what if you want, for example, this?

element.innerHTML = string.replace(/</g,"&lt;").replace(/>/g,"&gt;").replace(/\n/g,"<br>");

Use textcontent and white-space: pre-line;

It seems like everyone is on the same page about this now so I'll close it.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Hans5958 picture Hans5958  Â·  3Comments

GrahamSH-LLK picture GrahamSH-LLK  Â·  8Comments

WorldLanguages picture WorldLanguages  Â·  8Comments

Explosion-Scratch picture Explosion-Scratch  Â·  9Comments

WorldLanguages picture WorldLanguages  Â·  10Comments