Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Encode closing Tag #165

Open
jburghardt opened this issue Oct 9, 2019 · 8 comments · May be fixed by #167
Open

Encode closing Tag #165

jburghardt opened this issue Oct 9, 2019 · 8 comments · May be fixed by #167
Labels
bug help wanted

Comments

@jburghardt
Copy link

jburghardt commented Oct 9, 2019 •
edited
Loading

Currently encoding in the index.js only includes

const ENCODE = [
  ['&', '&'],
  ['>', '>'],
];

If a component is being rendered SSR and includes a property with a closing script tag,
the script tag in the SSrendered HTML will close the hypernova script.

<script type="application/json" data-hypernova-key="App" data-hypernova-id="....">
   <!-- {"props": ..., "title":"</script "}

which will throw an error in the JSON.parse method of the payload.

is there a reason closing tags are not encoded here ?
Following changes would suffice:

var ENCODE = [
['&', '&amp;'],
['>', '&gt;'],
['<', '&lt;']
];
@ljharb
Copy link
Collaborator

ljharb commented Oct 9, 2019

</script shouldn't close anything? you'd need </script>, and the > is escaped.

@jburghardt
Copy link
Author

<script with a blank after the t does close the hypernova script

@ljharb
Copy link
Collaborator

ljharb commented Oct 10, 2019

It seems like indeed </ specifically should be escaped.

@jburghardt
Copy link
Author

This is what could happen

<html>
   <head></head>
   <body>
   
   <script type="application/json" id="hypernova-app"><!-- {"props": {"message": "Evil user comment containing </script ", "foo": "bar"}} --></script>
    
   <script type="text/javascript">
  document.addEventListener('DOMContentLoaded', function () {
   window.alert(document.getElementById('hypernova-app').innerHTML);
   });
   </script>
  
 </body>
</html>

@jburghardt
Copy link
Author

It seems like indeed </ specifically should be escaped.

escaping just < should be enough.

@ljharb
Copy link
Collaborator

ljharb commented Oct 10, 2019

That will cause a lot more escaping, of all html tags, unnecessarily. We should only escape the pair.

@jburghardt jburghardt linked a pull request Oct 11, 2019 that will close this issue
Open
@jburghardt
Copy link
Author

Yep you are right, i updated the pull request.

ljharb pushed a commit to jburghardt/hypernova that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
569b540 
Fixes airbnb#165.
ljharb pushed a commit to jburghardt/hypernova that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
b5fd5c4 
Fixes airbnb#165.
ljharb pushed a commit to jburghardt/hypernova that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
fd46e37 
Fixes airbnb#165.
ljharb pushed a commit to jburghardt/hypernova that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
6e17e98 
Fixes airbnb#165.
ljharb pushed a commit to jburghardt/hypernova that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
a49fb59 
Fixes airbnb#165.
ljharb pushed a commit to jburghardt/hypernova that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
a56b7df 
Fixes airbnb#165.
ljharb pushed a commit to jburghardt/hypernova that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
05b2179 
Fixes airbnb#165.
ljharb pushed a commit that referenced this issue Mar 24, 2020
@ljharb
[Fix] escape closing </script tags
00c4b0f 
Fixes #165.
@csharplus
Copy link

@duoertai could you please take a look at this issue?

@csharplus csharplus assigned csharplus and unassigned csharplus May 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

escape closing tags jburghardt/hypernova
3 participants
@ljharb @jburghardt @csharplus