https://sports.yahoo.com/dailyfantasy is broken with Tor Browser 5.0 on Windows

added component::applications/tor browser owner::tbb-team priority::medium resolution::fixed severity::normal status::closed tbb-5.0-regression tbb-usability-website type::defect labels

Trac:
Summary: https://sports.yahoo.com/dailyfansty is broken with Tor Browser 5.0 on Windows to https://sports.yahoo.com/dailyfantasy is broken with Tor Browser 5.0 on Windows

It is somewhat surprising but this is indeed a Windows only issue. Testing on OS X 10.6.8 and Linux boxes is working fine. The first alpha that breaks is 5.0a3.

Browsing to https://sports.yahoo.com/dailyfantasy, I noticed that a window.Intl is undefined error appears in the JS console on Windows. This error is caused by our use of --without-intl-api in .mozconfig-mingw (our Windows build). So I wondered if this error might be causing the problem, but our TBB-4.5.3 builds also use --without-intl-api.

I then discovered that a polyfill.min.js script is loaded by the page in TBB-4.5.3 that compensates for the lack of a built-in window.Intl object. This file adds IntlPolyfill and Intl properties to the window object, such that Intl === IntlPolyfill returns true in the JS console. On TBB-5.0.2, the page attempts to load polyfill.min.js, but the server responds with an empty file (200 OK).

The request headers from the two browser versions are almost identical. One difference is the User-Agent, which is controlled by the general.useragent.override pref:

TBB-4.5.3 has Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Firefox/31.0
TBB-5.0.2 has Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Firefox/38.0

If I set the general.useragent.override in TBB-5.0.2 to empty, or to

Mozilla/5.0 (Windows NT 6.1; rv:38.0) Gecko/20100101 Firefox/31.0 or
Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Firefox/38.0, then the Yahoo server returns a non-empty polyfill.min.js and the links in https://sports.yahoo.com/dailyfantasy are no longer broken.

So there are two possible solutions to this bug: (1) cross-compile Firefox for Windows with the Intl API, and (2) change the User-Agent as above. Solution 1 may not be easy (see https://bugzilla.mozilla.org/show_bug.cgi?id=1019744#c19), but Solution 2 seems like an weird thing to do for an issue specific to this one site. On the other hand, since we have a habit of disabling dangerous new features, there might be an argument for not presenting a User-Agent that claims support for 38.0 features.

Thanks for taking this off of my plate and nice analysis! :) So, I agree that (1) is the ideal solution but it probably won't happen in the near future. I am not so happy about (2) as I bet there are a bunch of sites out there that break due to sending such a weird UA and sending none is very likely breaking a lot. The side behavior doesn't actually make sense to me as we already needed to disable ICU (i.e. the Internationalization API) in Tor Browser based on ESR 31. I propose contacting Yahoo to find out what this is all about first and we can decide whether we want to do something based on the response.

Trac:
Keywords: N/A deleted, TorBrowserTeam201509 added

Replying to gk:

Thanks for taking this off of my plate and nice analysis! :) So, I agree that (1) is the ideal solution but it probably won't happen in the near future. I am not so happy about (2) as I bet there are a bunch of sites out there that break due to sending such a weird UA and sending none is very likely breaking a lot.

Sorry I wasn't clear -- the UA I was thinking of sending is Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Firefox/31.0 which is the UA we were sending for TBB-4.5.3. The problem I see with sending the 38.0 UA is that it gives the false impression that we support all 38.0 features.

I agree we could try contacting Yahoo, though I haven't found any obvious way to do so.

Replying to arthuredelstein:

So there are two possible solutions to this bug: (1) cross-compile Firefox for Windows with the Intl API, and (2) change the User-Agent as above. Solution 1 may not be easy (see https://bugzilla.mozilla.org/show_bug.cgi?id=1019744#c19)

ICU can be cross-compiled (with some patches): https://build.opensuse.org/package/show/windows:mingw:win64/mingw64-icu https://apps.fedoraproject.org/packages/mingw-icu https://aur.archlinux.org/packages/mingw-w64-icu/

Replying to arthuredelstein:

Replying to gk:

Thanks for taking this off of my plate and nice analysis! :) So, I agree that (1) is the ideal solution but it probably won't happen in the near future. I am not so happy about (2) as I bet there are a bunch of sites out there that break due to sending such a weird UA and sending none is very likely breaking a lot.

Sorry I wasn't clear -- the UA I was thinking of sending is Mozilla/5.0 (Windows NT 6.1; rv:31.0) Gecko/20100101 Firefox/31.0 which is the UA we were sending for TBB-4.5.3. The problem I see with sending the 38.0 UA is that it gives the false impression that we support all 38.0 features.

I see. The interesting thing is that ICU was already a feature of ESR 31. And switching back to ESR 31 leads very likely to users showing up here complaining about websites saying they should upgrade their browser...

I agree we could try contacting Yahoo, though I haven't found any obvious way to do so.

I tried for quite some while to find a way to send them something without the need for me to have a Yahoo account. I have a case number now! We'll see if that leads somewhere.

I guess what I'll do is spending some cycles on the proposed cross-compilation in the hope Jacek is fixing for us the remaining things once I got that somewhat working. :)

Yahoo wrote back:

I apologize for the inconvenience you have had with viewing Daily Fantasy on the Tor browser. Unfortunately, there isn't a tech that is available to speak to you about this. I would suggest to please use the most current version of Firefox non beta and this will resolve your issue.

That settles it I guess.

What do we think about the patches the cypherpunk linked in comment:7?

Replying to mikeperry:

What do we think about the patches the cypherpunk linked in comment:7?

Cross-compiling ICU outside of the way Firefox is doing it seems to be working for me even without the patches. Just the Firefox thing is breaking and I am working on it in #13419 (moved). So, I guess the patches won't help directly. But maybe they will help me understand what goes wrong in the Firefox case.

If we can't get ICU to build, another alternative is to write a stub (ideally in C++, but as a last resort in Torbutton) to fill in window.Intl with a simple object on Windows. I'm not sure I'll have time to do this myself, though.. :/

Kathy and I looked at this a little with an eye toward adding an Intl object. It looks like for the sports.yahoo.com page, minimally we would need to implement Intl.NumberFormat() and Intl.DateTimeFormat() and both of those would need to return an object that had a format() method. We do not have more time to work on this today, but will look at it again tomorrow unless someone beats us to it. The best solution would be to solve the cross-compile issues, but I suspect that is difficult too.

Trac:
Cc: N/A to brade, mcs

I remembered I had written a function that injects scripts into content documents in Firefox, so I tried using it to inject the Intl.js polyfill from https://github.com/andyearnshaw/Intl.js/.

Here's the patch: https://github.com/arthuredelstein/torbutton/commit/16874+1

Obviously, a fix for #13419 (moved) should be preferred to this approach. Nevertheless, this patch does fix the issue with https://sports.yahoo.com/dailyfantasy and it provides a presumably faithful implementation of Intl.NumberFormat and Intl.DateTimeFormat.

I'm unfamiliar with the new Services.scriptloader.loadSubScript API, but looking it up, it seems common for people to use https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.Sandbox with it still, to cause the script to evaluate with content privileges..

Are you sure that what you did with XPCNativeWrapper.unwrap() instead is safe there?

There's two main sources of risk when doing stuff like this:

Adding Intl.js from chrome could cause the Intl.js expandos to actually have chrome privileges inside the content window. This seems unlikely, but again I'm not sure.
The polyfill itself may be running with chrome privs when installing itself, which means that if it touches an unwrapped window property, it may be induced to execute content code (in the form of a getter/setter, perhaps) that way, in a privileged setting.

For some reason, I can't seem to find any documentation on either of these risks. Perhaps that's because this new API is magically safe no matter how you use it? Maybe this is a question for #security on irc.mozilla.org, and/or some testing?

Replying to mikeperry:

I'm unfamiliar with the new Services.scriptloader.loadSubScript API, but looking it up, it seems common for people to use https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.Sandbox with it still, to cause the script to evaluate with content privileges..

Are you sure that what you did with XPCNativeWrapper.unwrap() instead is safe there?

I don't why I had XPCNativeWrapper.unwrap() there -- I've removed it.

There's two main sources of risk when doing stuff like this:

Adding Intl.js from chrome could cause the Intl.js expandos to actually have chrome privileges inside the content window. This seems unlikely, but again I'm not sure.

The polyfill itself may be running with chrome privs when installing itself, which means that if it touches an unwrapped window property, it may be induced to execute content code (in the form of a getter/setter, perhaps) that way, in a privileged setting.

The documentation at https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Reference/Interface/mozIJSSubScriptLoader#loadSubScript%28%29 claims that "The loaded script is executed with the principal associated with the target object." So that's at least promising.

First I confirmed that the principal for the contentWindow object is non-chrome:

> Components.utils.getObjectPrincipal(gBrowser.selectedBrowser.contentWindow)
XPCWrappedNative_NoHelper { URI: XPCWrappedNative_NoHelper, origin: "https://arthuredelstein.github.io", jarPrefix: "", baseDomain: "arthuredelstein.github.io", appStatus: 0, appId: 0, isInBrowserElement: false, unknownAppId: false, isNullPrincipal: false }

Then for a more direct test, I added the line

console.log("Components.interfaces:", Components.interfaces);

to the top of Intl.js. In that case, every loaded page logs an error:

ReferenceError: Components is not defined

I didn't immediately think of any further tests that might approach the question in another way.

For some reason, I can't seem to find any documentation on either of these risks. Perhaps that's because this new API is magically safe no matter how you use it? Maybe this is a question for #security on irc.mozilla.org, and/or some testing?

You're right that we should be very careful about this. I've posted a question there. In the meantime, here's the new version without XPCNativeWrapper.unwrap:

https://github.com/arthuredelstein/torbutton/commit/16874+2

I suppose one alternative would be to insert a <SCRIPT src="re[/...">](/...">`) element to the content page's DOM once it has started loading.

Bobby Holley had a good suggestion here: https://groups.google.com/forum/#!topic/mozilla.dev.platform/OWGw4Ep7AEM

So here's a new version of the patch that uses contentWindow.eval(...) instead of Services.scriptloader.loadSubScript: https://github.com/arthuredelstein/torbutton/commit/16874+3

Moving Tor Browser tickets to October 2015.

Trac:
Keywords: TorBrowserTeam201509 deleted, TorBrowserTeam201510 added

Trac:
Keywords: TorBrowserTeam201510 deleted, N/A added

Fixed by #13419 (moved) which will be available in 6.0a5.

Trac:
Reviewer: N/A to N/A
Status: new to closed
Severity: N/A to Normal
Sponsor: N/A to N/A
Resolution: N/A to fixed

closed

moved to tpo/applications/tor-browser#16874 (closed)

https://sports.yahoo.com/dailyfantasy is broken with Tor Browser 5.0 on Windows

Child items 0

Activity