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

XSS vulnerability in the admin tool #214

Closed
GUI opened this issue Apr 10, 2015 · 1 comment
Closed

XSS vulnerability in the admin tool #214

GUI opened this issue Apr 10, 2015 · 1 comment

Comments

@GUI
Copy link
Member

GUI commented Apr 10, 2015

I stumbled across some cross-site-scripting (XSS) vulnerabilities in the admin tool related to how we weren't escaping user account information when displaying it (mainly in the table listings).

I've stepped through all the places in the admin where we display user information, and I think all of these issues should be properly escaped and tested now. I've also ensured that all other similar content is escaped, even if it's not necessarily user-submitted content, just to be doubly safe.

Most of this stemmed from my bad assumption that DataTables escaped content is was displaying from JSON sources, but that is not the case. This didn't use to be an issue due to how the tables were previously generated, but since more of the admin tables have been switched over to use DataTables, this is why this had become an issue.

To ensure that this vulnerability had not actually been exploited, I've also audited all of our existing user data to ensure that nobody had previously taken advantage of this vulnerability without us knowing. The good news is that there isn't any fishy content in our database, so I don't think we have to worry about anyone having done anything malicious.

This was fixed by NREL/api-umbrella-web@f53a9fb and NREL/api-umbrella-web@bcc0e92

@GUI
Copy link
Member Author

GUI commented Apr 11, 2015

One additional related change I meant to make before: I've adjusted the session cookie that the app sets to enable two settings to better secure things moving forward:

  • The httponly flag, which should generally help reduce the impact of XSS issues if they do happen to crop up again (by making the cookie inaccessible to javascript).
  • The secure flag, which should help eliminate session sidejacking on non-https content (since we still support non-https for some of our public website and APIs).

Fixed by NREL/api-umbrella-web@5d095bc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant