Ayende @ Rahien

Refunds available at head office

Resolving cross site scripting issues.

I got a bug report about the following in the admin UI for RavenDB.

image

As you can imagine, this is certainly something that we would like to avoid, but there is a problem. How the hell do you find the problem?

I mean, obviously we are encoding the value when we present it to the user, since I can see it on the UI. But it is still running, so I am doing something bad here. But I don’t feel like traversing through a mountain of JavaScript to find out exactly where this is happening. Luckily, we don’t have to, we can use the XSS itself to help it localize it:

image

And given that, we can get directly to the actual fault:

image

And fixing that is a snap.

Comments

Marcus
09/18/2010 10:49 AM by
Marcus

Excuse my incompetence but can you elaborate the connection between the yellowish debbuger variable and the javascript line of code?

Mike Scott
09/18/2010 11:44 AM by
Mike Scott

Marcus

Do you mean the tooltip that displays the value of the children variable? Because the value isn't HTML encoded, any javascript will be inserted straight into that <span as is and thus will be executed on the browser - in this case a debugger breakpoint and then a call to the alert() function.

Mike Scott
09/18/2010 11:49 AM by
Mike Scott

Oren, sweet use of the debugger breakpoint!

I also wanted to suggest chaining the calls to append() in your jquery to prevent doing the $(childDiv) lookup multiple times, for example.

Ken Egozi
09/18/2010 11:49 AM by
Ken Egozi

you should look into some kind of string templating instead of the concatenations

EJS is a great pick. it gives you a syntax similar to php/asp/ERB that is extremely familiar.

so you'd be creating the markup in using a view, then transferring the markup built to jQuery to be inserted to the DOM

Marcus
09/18/2010 11:56 AM by
Marcus

@Mike: no thats not what I mean,

I mean the other yellow-marked word 'debugger', how does that bring him to the line of javascript code?

tobi
09/18/2010 12:14 PM by
tobi

Nice technique. Are 100% percent sure that the "key" is already encoded as well? At the very least it can contain umlauts which have to be html encoded (or you might find yourself debugging a much harder problem a year in the future).

Andrew
09/18/2010 12:21 PM by
Andrew

@Marcus: At least in Firebug when the javascript 'debugger;' statement is encountered; this simulates a breakpoint.

Marcus
09/18/2010 12:24 PM by
Marcus

@Andrew: thanks thats explains it

He seems to be using Chrome though, according to the first image,

maybe Chrome has that functionality too.

Ayende Rahien
09/18/2010 12:47 PM by
Ayende Rahien

Marcus,

That is the same behavior in all browsers.

Felipe Fujiy
09/18/2010 06:54 PM by
Felipe Fujiy

I didn´t understand why that string is show in UI and executing too.

Luke
09/18/2010 07:25 PM by
Luke

Oops...my bad Oren.

Steve Gentile
09/18/2010 08:55 PM by
Steve Gentile

As above that JsonDiv is getting wrapped twice

It was recommended to me to use $jsonDiv as a variable name to make it clear that it was already a wrapped jQuery object

Jonas
09/19/2010 07:58 PM by
Jonas

Fixing just one instance of a problem is a sin You definitely have more problems like that and one alone is enough to hijack another user's credentials.

Miranda
09/19/2010 08:16 PM by
Miranda

Or... you could just encode everything? I'm not sure what this is all about.

Michael Fever
09/21/2010 02:06 AM by
Michael Fever

Not all browsers show it the same way, but they will direct you to the line # of the problem. Chrome has the best devtools .. worth checking out.

Comments have been closed on this topic.