Potential stored xss issue?

[apitts]

Here:

mean/modules/core/server/views/layout.server.view.html

Line 48 in 3a4c51a

var user = {{ user | json | safe }};

I am thinking about stored xss in the user object (e.g. signup with a script as a user name).

Let me know if I've missed where this would be filtered out elsewhere please.

[mg1075]

I am curious as to why that was left that in there because it looks like code testing rather than production code.

Also worth noting that all these properties of the user object will be exposed by angular if the user pries:

"_id":"5661d490ae5c35b85ce9f639",
"displayName":"john smith",
"provider":"local",
"username":"john.smith",
"__v":0,
"created":"2015-12-04T17:59:44.450Z",
"roles": ["user"],
"profileImageURL":"modules/users/client/img/profile/default.png",
"email":"john.smith@notgivingmyemail-because.org",
"lastName":"smith",
"firstName":"john"

Earlier I raised this issue in gitter, and @mleanos replied:

@mg1075 Ok. I must have been thinking about something else that isn't present in Production. > Someone here know's, I'm sure. You can open an issue, if you haven't already, to get to the > bottom of it.
I don't see much of a security rish with this though. Nobody else, but the logged in user can see > this data. It's their own data right? I think there could be an argument to remove the roles & > provider fields (maybe _id too)

[apitts]

Thanks @mg1075. I am not up to speed on v0.4 I'm afraid but I know in previous versions the embedded user object was referenced directly client-side.

To the extent that people are using mean.js to build very simple apps then I don't think it is a serious security issue....but the moment you create a view that exposes another users' username (i.e. pretty much any social or platform-type app) I believe there is a potential issue.

[apitts]

I think perhaps here:
https://github.com/meanjs/mean/blob/master/modules/users/client/services/authentication.client.service.js#L7

Which is then referenced in the header at a minimum.

[mg1075]

Yep...

[apitts]

Correction: views with other usernames will of course be sanitized by angular...so, yeah, probably not a major security issue.

[lirantal]

I don't see a problem with these fields. It shows your username or e-mail to you, so what?
On the stored XSS that's another story - were you actually able to introduce this kind of vulnerability?

[apitts]

@lirantal I have sent you an email on the XSS topic.

[lirantal]

thanks, so where should this be fixed?

1. on the backend to sanitize certain strings? usernames are usually more "open" in the sense of allowing all kinds of utf8 chars
2. on the frontend to always sanitize strings before outputing?

so (2) is an obvious fix no matter what and we can already provide a PR for. I believe Angular has it's own built in filters for that.

as for (1) that's a more open question, usually data is saved on the db "as is"

[apitts]

I believe if we can do 2 that should be enough. Issue is I believe we have escaped swig's sanitization. I for one am not certain how you would apply angular sanitization to a swig object...no doubt it's possible though.

[simison]

I feel like it should be stated clearly at this point — if someone manages to produce an actual XSS vulnerability — how should they disclose that information? Openly here or via emails directly to @lirantal?

@apitts that swig object is visible only to authenticate user him/herself. For others everything is visible only via Angular, which has its own escaping methods as mentioned.

[ilanbiala]

@simison email to everyone on the team is best.

[lirantal]

e-mails are best for now, yes.

[apitts]

@simison Yes - I agree. That is what I was trying to get at with my correction above but you have stated it more clearly. I can envision a situation where some other user-generated content is stored across users...though that would be pretty rare.

[lirantal]

Hi @apitts

Getting back to this - I did see that the script tags are breaking the HTML and messing it up but I don't see any popups, though I bet it's easier to still attack it via XSS with a proper query.

Were there any specific areas in the view that you noticed suffering from the XSS?