Using innerHTML, and what are security concerns?
Asked Answered
V

1

8

I've been told it's dangerous to use innerHTML in my JS codes. and I did understand why, but can someone tell me if there is a difference between these two ways of using it?

FIRST:

const onePersonArticle = create('article');
onePersonArticle.className = 'one-person';
const content = ` <label class='info-labels' for="name">NAME: </label><label id='name' class='data-labels'>${member.name}</label>
    <label class='info-labels' for="phone">PHONE: </label><label id='phone' class='data-labels'>${member.phone}</label>
    <label class='info-labels' for="code-wars">CODEWARS: </label><label id='code-wars' class='data-labels'>${member.cwb} - ${member.cwa}</label>
    <label class='info-labels' for="free-code-camp">FREECODECAMP: </label><label id='free-code-camp' class='data-labels'>${member.fccb} - ${member.fcca}</label>
    <label class='info-labels' for="notes">NOTES: </label><label id='notes' class='data-labels'>${member.notes}</label>
    <span class="person-buttons"><button type="button" name="button" class="button delete-button">⌘</button>
    <button type="button" name="button" class="button edit-button">✎</button></span>`;
onePersonArticle.innerHTML = content;

SECOND:

const onePersonArticle = create('article');
onePersonArticle.className = 'one-person';
onePersonArticle.innerHTML =
  ` <label class='info-labels' for="name">NAME: </label><label id='name' class='data-labels'>${member.name}</label>
    <label class='info-labels' for="phone">PHONE: </label><label id='phone' class='data-labels'>${member.phone}</label>
    <label class='info-labels' for="code-wars">CODEWARS: </label><label id='code-wars' class='data-labels'>${member.cwb} - ${member.cwa}</label>
    <label class='info-labels' for="free-code-camp">FREECODECAMP: </label><label id='free-code-camp' class='data-labels'>${member.fccb} - ${member.fcca}</label>
    <label class='info-labels' for="notes">NOTES: </label><label id='notes' class='data-labels'>${member.notes}</label>
    <span class="person-buttons"><button type="button" name="button" class="button delete-button">⌘</button>
    <button type="button" name="button" class="button edit-button">✎</button></span>`;
container.appendChild(onePersonArticle);

and now, if both ways are vulnerable to code injectios, is there anyother way to implement HTML tags in the same way as a string into HTML Pages?

Violetavioletta answered 22/12, 2017 at 18:11 Comment(2)
both ways are not recommended. You will probably face some problems to work with childNodes into your elementInfantry
For further information, see DOM based XSS Prevention Cheat Sheet.Millar
D
13

Both are safe if you are in complete control of all aspects of the string being used.

Having said that, in general, .innerHTML is for small fragments of HTML to be inserted and parsed into a document, not large strings (as you have here) because it becomes a nightmare to support.

When you have large amounts, such as what you are showing here, the HTML <template> and JavaScript document.importNode() may be preferable as you have a more simple way of injecting content as a series of DOM nodes that lend themselves to easier manipulation via an API than a bunch of string concatenations. This allows you to have the elements created dynamically and then you can populate them using .textContent, rather than .innerHTML, which removes the security issues since the text is not parsed as HTML.

Delvalle answered 22/12, 2017 at 18:17 Comment(4)
Agreed. Also see difference between innerHTML and createTextNode.Millar
Agreed, but it's a bit misleading to say that both are safe. Most probably both of the examples in the question are vulnerable to XSS.Marjie
@GaborLengyel It wasn’t misleading because I said they are both safe only when you are in complete control of the string.Delvalle
@ScottMarcus I understand, but to somebody that doesn't understand XSS or vulnerabilities in general, you said it's fine, when it is very probable that the above code is vulnerable. "Complete control of all aspects of the string" is too vague and doesn't mean much to security-unaware developers... I don't mean to offend or debate though, strictly speaking your answer is very much correct, I just worked with many devs and this is my experience. :)Marjie

© 2022 - 2024 — McMap. All rights reserved.