ampproject / worker-dom

The same DOM API and Frameworks you know, but in a Web Worker.
Apache License 2.0
3.21k stars 152 forks source link

Script tags created by innerHTML should not execute when synced to main thread #311

Open jridgewell opened 5 years ago

jridgewell commented 5 years ago

After https://github.com/ampproject/worker-dom/issues/283, we should fix the thread mutation sync so that script elements don't execute on the main thread:

div.innerHTML = `
  <script>alert("I won't execute");</script>
`;

When we do this, we should also ensure that regularly created script elements do execute:

const s = document.createElement('script');
s.textContent = `alert("I will execute");`;
div.appendChild(s);

One way to do this would be to add a _disabled flag to the node. When using innerHTML, script._disabled = true. When it's being recreated on the main thread, we can do the following:

const throwaway = document.createElement('div');
throwaway.innerHTML = `<script></script>`;
const script = throwaway.firstChild;

A script created like this will not be able to execute.

kristoferbaxter commented 5 years ago

This looks like a good addition for the safe mode.

dreamofabear commented 5 years ago

Safe mode/DOMPurify won't allow injection of script tags that execute code anyways, so this is only relevant to the "unsafe" mode.

Tangent: Should we rename "safe" mode to "AMP" mode?