GlennHS/Scrum-Helper

Potential to inject HTML via input

ssddanbrown opened this issue · 1 comments

Describe the bug
HTML can be added/altered to the page via the form inputs which allows potential of breaking. Even just the use of quote marks and <> arrows may cause breakages. This is due to the direct usage of variables when building out HTML strings such as here:
https://github.com/ssddanbrown/Scrum-Helper/blob/d4e37233917e64f926ce5b1f06d0a80ba5c65356/index.html#L234

Instead, text-only methods such as .textContent = 'cat' and jQuery's .text('cat') should be used where dealing with user input, targeted as specific elements. You could still build the template/original HTML content as you've done now, but then target specific areas with those methods to add the dynamic input text elements safely.

On an semi-related note, if you didn't want a large string of HTML within your JS, and instead maybe wanted to keep that nearer it's intended location within the page HTML, the HTML template tag can come in handy:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template
Basically provides read-only fragments of document you can clone. You could have a template for task element then clone it out within your current addTaskToDOM, Then apply the dynamic parts that change upon the clone.

To Reproduce
Steps to reproduce the behavior:

  1. Add a new task.
  2. Set Clarizen name to:
    "><h1>donkey</h1><img src="https://cataas.com/cat/cute" style="position:fixed; top: 0; left:0; width: 100vw; height: 100vh; z-index:500"><h1 id="
    
  3. Generate message or move the task up/down where another task exists.
  4. See cats

Expected behavior
It should not be possible to manipulate the page HTML via inputs.

Desktop (please complete the following information):

  • OS: Fedora 34
  • Browser: Firefox 93

Additional context
Not a massive concern here to be honest since the input content is not shared with others but become a much larger concern if data was shared within a multi-user context since people could inject scripts or other sketchy content.

As someone who's a massive fan of cyber security and SQL injection attacks, this should have really been noticed by me sooner. Thanks so much for bringing it up and I'll aim to have it fixed ASAP. Also thanks for giving such clear, thought out explanations with your comments, it really helps me :)