Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

users should not be able to inject html #1

Closed
bboyle opened this issue Jun 8, 2014 · 2 comments
Closed

users should not be able to inject html #1

bboyle opened this issue Jun 8, 2014 · 2 comments

Comments

@bboyle
Copy link

bboyle commented Jun 8, 2014

Steps to reproduce:

  1. visit the app: http://jtme.github.io/ToDolist/
  2. add a new item: <span style="color: fuchsia;">hello</span>
  3. observe that the html is added (the colour is applied)

Users would be able to interfere with the app through html injection (either malicious or accidental), for example entering </li> hello will close the list item and break the layout, and entering <script src="..."></script> can inject foreign script which will be executed.

@bboyle
Copy link
Author

bboyle commented Jun 8, 2014

Review where the input is parsed: https://github.com/jtme/ToDolist/blob/gh-pages/script.js#L17
var add_value = $('input').val();

And inserted: https://github.com/jtme/ToDolist/blob/gh-pages/script.js#L18
var new_item = $("<li> <span class=\"item\">" + add_value + "</span></li>");

@bboyle
Copy link
Author

bboyle commented Jun 8, 2014

I suggest something like this:

var user_input = $('input').val();
var clean_input = $('<div>').text(user_input).html();
var new_item = $("<li> <span class=\"item\">" + clean_input + "</span></li>");

@jtme jtme closed this as completed Jun 16, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants