Kevin Powell - Community•2w 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•2w 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•2w ago

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

glutonium•2w ago

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

Untitled

...

glutonium•2w ago

welcm

snxxwyyOP•2w ago

Thank you for taking the time to do that.

glutonium•2w ago

welcome

ἔρως•2w ago

how about you use svg symbols? or do this

snxxwyyOP•2w ago

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

glutonium•2w ago

probably <use> and <symbol>

ἔρως•2w ago

yup you <use> the <symbol>

рари•2w 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•2w ago

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

ἔρως•2w 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

рари•2w ago

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

snxxwyyOP•2w ago

In what way?

ἔρως•2w 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

рари•2w ago

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

snxxwyyOP•2w ago

Oh if you try to animate between the two icons?

ἔρως•2w ago

no, if you try to animate at all

snxxwyyOP•2w ago

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

glutonium•2w 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

ἔρως•2w 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•2w ago

haha xD

ἔρως•2w 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•2w ago

i was typing on mobile so no red squiggly xD

ἔρως•2w ago

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

glutonium•2w ago

wdym? i do have autocorrect off

Gaming

Programming

icon swap | InnerHTML security risks