Kevin Powell - Community•2mo ago

icon swap | InnerHTML security risks

Hey, if you had a button with an icon in it, let’s say it’s a theme toggle, and your icon switches from a sun to a moon when you click, the way I’d imagine doing that is by setting the button’s innerHTML to a different svg. I’ve heard this poses security risks though since users can manipulate the data in there. What would a better alternative be for swapping out content like that?
Using adjacentHTML doesn’t seem like it would fit the use case since this adds the content but doesn’t replace it. I know you could then remove the other icon but I feel you could do the whole thing in one swoop without doing that? I may be wrong. I believe there was a method called .replaceWith or something along those lines but I’d assume you would put html in there too so I’d imagine there’s security risks with that as well, and I can’t imagine using a template for something small like this is worth it? Please correct me if I’m wrong with any of this. I’d appreciate any insight, thanks in advance.

27 Replies

glutonium•2mo ago

I'd just put both svgs inside there and just simply toggle display accordingly like if the default theme is light then by default the moon svg will be display: none and when u click the btn the display none gets removed from the moon and gets added to the sun

snxxwyyOP•2mo ago

Oh okay I see, that makes more sense, thank you.

glutonium•2mo ago

https://codepen.io/-bloop-/pen/OJKaRrq

Untitled

...

glutonium•2mo ago

welcm

snxxwyyOP•2mo ago

Thank you for taking the time to do that.

glutonium•2mo ago

welcome

ἔρως•2mo ago

how about you use svg symbols? or do this

snxxwyyOP•2mo ago

Is that the <use> tag? And you mean just swap the href for it on click?

glutonium•2mo ago

probably <use> and <symbol>

ἔρως•2mo ago

yup you <use> the <symbol>

рари•2mo ago

Briefly: 1) Class Based Approach as in Codepen below is good 2) Approach like this

const toggleTheme = (button) => {
  const sunSvg = `<svg class="sun-icon">...</svg>`;
  const moonSvg = `<svg class="moon-icon">...</svg>`;
  
  button.innerHTML = button.innerHTML.includes('sun-icon') ? moonSvg : sunSvg;
};

document.querySelector('.theme-toggle').addEventListener('click', toggleTheme);

const toggleTheme = (button) => {
  const sunSvg = `<svg class="sun-icon">...</svg>`;
  const moonSvg = `<svg class="moon-icon">...</svg>`;
  
  button.innerHTML = button.innerHTML.includes('sun-icon') ? moonSvg : sunSvg;
};

document.querySelector('.theme-toggle').addEventListener('click', toggleTheme);

Can lead to some XSS and DOM manipulation risk 3) replaceChild is wacky cause it creates a new DOM element on each toggle

const createIcon = (type) => {
  const svg = document.createElementNS("http://www.w3.org/2000/svg", "svg");
  svg.setAttribute("viewBox", "0 0 24 24");
  svg.classList.add(`${type}-icon`);
  // paths and attrs
  return svg;
};

const toggleTheme = (button) => {
  const currentIcon = button.children[0];
  const newIcon = createIcon(
    currentIcon.classList.contains('sun-icon') ? 'moon' : 'sun'
  );
  button.replaceChild(newIcon, currentIcon);
};

document.querySelector('.theme-toggle').addEventListener('click', toggleTheme);

const createIcon = (type) => {
  const svg = document.createElementNS("http://www.w3.org/2000/svg", "svg");
  svg.setAttribute("viewBox", "0 0 24 24");
  svg.classList.add(`${type}-icon`);
  // paths and attrs
  return svg;
};

const toggleTheme = (button) => {
  const currentIcon = button.children[0];
  const newIcon = createIcon(
    currentIcon.classList.contains('sun-icon') ? 'moon' : 'sun'
  );
  button.replaceChild(newIcon, currentIcon);
};

document.querySelector('.theme-toggle').addEventListener('click', toggleTheme);

4) SVG sprite is cool just like 1)

<svg xmlns="http://www.w3.org/2000/svg" style="display: none;">
  <symbol id="icon-sun" viewBox="0 0 24 24">
    <path d="[path]"/>
  </symbol>
  
  <symbol id="icon-moon" viewBox="0 0 24 24">
    <path d="[path]"/>
  </symbol>
</svg>


<button class="theme-toggle" aria-label="Toggle theme">
  <svg class="icon icon-sun" width="24" height="24">
    <use href="#icon-sun"/>
  </svg>
  <svg class="icon icon-moon hidden" width="24" height="24">
    <use href="#icon-moon"/>
  </svg>
</button>

<svg xmlns="http://www.w3.org/2000/svg" style="display: none;">
  <symbol id="icon-sun" viewBox="0 0 24 24">
    <path d="[path]"/>
  </symbol>
  
  <symbol id="icon-moon" viewBox="0 0 24 24">
    <path d="[path]"/>
  </symbol>
</svg>


<button class="theme-toggle" aria-label="Toggle theme">
  <svg class="icon icon-sun" width="24" height="24">
    <use href="#icon-sun"/>
  </svg>
  <svg class="icon icon-moon hidden" width="24" height="24">
    <use href="#icon-moon"/>
  </svg>
</button>

const toggleTheme = () => {
  document.querySelector('.icon-sun').classList.toggle('hidden');
  document.querySelector('.icon-moon').classList.toggle('hidden');
};

document.querySelector('.theme-toggle').addEventListener('click', toggleTheme);

const toggleTheme = () => {
  document.querySelector('.icon-sun').classList.toggle('hidden');
  document.querySelector('.icon-moon').classList.toggle('hidden');
};

document.querySelector('.theme-toggle').addEventListener('click', toggleTheme);

snxxwyyOP•2mo ago

Thank you for the suggestion Thank you for taking the time to write that, it definitely helps out

ἔρως•2mo ago

option 5: change the href of <use> which is the easiest of all also, do not set display: none on svgs that royally fucks all animations, and they dont play anymore

рари•2mo ago

Didn't play with the sprites enough, thanks for the insights

snxxwyyOP•2mo ago

In what way?

ἔρως•2mo ago

the animations dont work you have to set the width and height to 0, then use visibility ive had that problem before, and the display: none was the cause

рари•2mo ago

so many little details, that's why I really love the frontend

snxxwyyOP•2mo ago

Oh if you try to animate between the two icons?

ἔρως•2mo ago

no, if you try to animate at all

snxxwyyOP•2mo ago

Any animation on the page? Or just regarding the button/svg

glutonium•2mo ago

ya true if u want to animate then display won't work display is not animateable i think i just made up that word "animateable" lol

ἔρως•2mo ago

just on any svg that uses the symbols from a display: none svg lol you wouldnt have invented if you spelled it correctly: https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_animated_properties

glutonium•2mo ago

haha xD

ἔρως•2mo ago

don't worry, it happens but didn't the red squiggly made you curious about if the word exists and you misspelled it? 🤔

glutonium•2mo ago

i was typing on mobile so no red squiggly xD

ἔρως•2mo ago

🤔 no duck when you want to type ... pickles?

glutonium•2mo ago

wdym? i do have autocorrect off

Gaming

Programming

icon swap | InnerHTML security risks