Opened 6 years ago

Closed 5 years ago

#5922 closed defect (fixed)

Greg's browserfeedwriter javascript finds TBB's full path on Windows

Reported by: arma Owned by: Sebastian
Priority: Very High Milestone:
Component: Applications/Tor bundles/installation Version:
Severity: Keywords: tbb-fingerprinting
Cc: erinn, Sebastian, g.koppen@…, Shondoit Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

http://pseudo-flaw.net/tor/torbutton/browserfeedwriter-error.html

Shondoit went there using TBB on Windows and it said
"found location: "file:///[...]/Tor%20Browser/FirefoxPortable/App/Firefox/components/FeedWriter.js" where [...] is his install location.

Reported at https://blog.torproject.org/blog/new-tor-browser-bundles-17#comment-15627

Child Tickets

Change History (23)

comment:1 Changed 6 years ago by erinn

Cc: erinn Sebastian added

comment:2 Changed 6 years ago by gk

Cc: g.koppen@… added

comment:3 Changed 6 years ago by mikeperry

Component: TorBrowserButtonFirefox Patch Issues
Keywords: tbb-linkabilility added
Priority: majorcritical

FYI: For whatever reason, Mac and Linux use resource urls in JS exceptions, not filesystem paths. Pretty sure this was a huge concern for Mozilla back in the Firefox 2.0 days, and it got fixed everywhere.. Or at least, so we thought. Not sure what the hell is up with Windows.. If it was always broken, or if someone snuck in a helpful new "debugging feature" into Rapid Release at some point.

Am currently digging around for things that implement nsIException to try to find the culprit. I'll see if I can find the mercurial blame logs, too.

comment:4 Changed 6 years ago by mikeperry

Keywords: tbb-fingerprinting MikePerry201205 added; tbb-linkabilility removed

Classes that implement nsIException: nsBaseDOMException, nsXPCException, nsException.

The particular error Greg generates appears to come from nsXPCException. It gets its location set in nsXPCException::Initialize() via an nsIStackFrame.

nsBaseDOMException appears to be a wrapper for another nsIException instance (possibly nsXPCException).

I cannot find any implementation of nsException, other than a header file with a class definition.

comment:5 Changed 6 years ago by mikeperry

nsIStackFrame is implemented by XPCJSStackFrame, which appears to get the filename from either callers of XPCJSStackFrame::CreateStackFrameLocation() or from JS_GetScriptFilename().

comment:6 Changed 6 years ago by mikeperry

And.. JSScript gets the filename from the ByteCodeEmitter's Parser object...

comment:7 Changed 6 years ago by mikeperry

Component: Firefox Patch IssuesTor bundles/installation
Owner: changed from mikeperry to erinn

Aha! Neutron on IRC confirms that these paths *are* santized in Mozilla's official Firefox builds.

So this is due to something different between how we build Firefox for Mac and Linux vs our Windows builds.

comment:8 Changed 6 years ago by mikeperry

Keywords: MikePerry201205 removed

Btw: My money is on the FirefoxPortable cruft causing this issue. Other than that, I have no further insight into this bug, as I have no idea how our Windows build process works.

comment:9 in reply to:  8 Changed 6 years ago by Shondoit

Cc: Shondoit added

Replying to mikeperry:

Btw: My money is on the FirefoxPortable cruft causing this issue.

I just tried the official PortableApps Firefox and it sanitizes the paths too.
Also, this issue is not introduced after manually installing Torbutton in the Portable Firefox.

comment:10 Changed 6 years ago by Sebastian

Issue is still triggered after uninstalling torbutton, https everywhere and noscript

comment:11 in reply to:  8 ; Changed 6 years ago by Sebastian

Replying to mikeperry:

as I have no idea how our Windows build process works.

what do I have to do so that you will never need to say that again?

comment:12 Changed 6 years ago by Sebastian

A firefox without the patches has the same issue, even when ran from the build directory - no portablefirefox contamination possible at that point.

comment:13 Changed 6 years ago by Sebastian

This happens because firefox is not installed. When building a firefox installer and using that one, the path is properly obfuscated. Since it also doesn't happen for Portable Firefox, we should figure out what's different and how they manage to avoid leaking the path here.

comment:14 Changed 6 years ago by Sebastian

Aha. we can "make package" which might help here. Now to figure out how to actually integrate it into the build process

comment:15 Changed 6 years ago by Sebastian

Owner: changed from erinn to Sebastian
Status: newassigned

comment:16 in reply to:  14 Changed 6 years ago by arma

Replying to Sebastian:

Aha. we can "make package" which might help here.

Intriguing. I wonder what else 'make package' changes.

comment:17 in reply to:  11 Changed 6 years ago by mikeperry

Replying to Sebastian:

Replying to mikeperry:

as I have no idea how our Windows build process works.

what do I have to do so that you will never need to say that again?

Heh. Is my plan of annoying you to death by being a broken record working a little to well? ;)

I basically want a short, sweet document for bootstrapping a VM and building TBB, like you did for https://gitweb.torproject.org/torbrowser.git/tree/master:/docs/buildmachine_setups/osx.txt for Windows and Linux, too.

I know you're working on it. It's just that every time I run into a build-related issue, I am frustrated by the fact that I (and more importantly, others) have no idea how to help.

comment:18 Changed 6 years ago by Sebastian

Status: assignedneeds_review

branch bug5299 in my repo has a fix. The second patch is pretty ugly, but it seems to work. We should fix this better once we have gotten rid of the dynamic part of the firefox build dir (the hostname/arch stuff behind the obj-)

comment:19 Changed 6 years ago by Shondoit

Branch 'bug5922' in my repo has a greatly sanitized version. I propose that one for review.

comment:20 Changed 6 years ago by mikeperry

It looks like this just got merged? Is there a reason we chose Sebastian's version?

comment:21 in reply to:  20 Changed 6 years ago by Shondoit

Replying to mikeperry:

It looks like this just got merged? Is there a reason we chose Sebastian's version?

Sebastian's version doesn't work for clean build since it's missing the 'build' target.
Besides that, it copies along a jsshell_win32.zip file since it resides in the same directory as the finished package.

My branch copies directly from the staging folder instead of packing the staging folder and unpacking the resulting zip file.
More comments can be found in ./src/current-patches/mozilla-build/start-msvc.patch

I added the 're-bug5922' branch to my repo which is a rebased version of 'bug5922'

comment:22 Changed 6 years ago by Sebastian

A fixed version is now out. It uses my version of the patch however, so I'm keeping this bug open for now so we can go with Shondoit's version later

comment:23 Changed 5 years ago by erinn

Resolution: fixed
Status: needs_reviewclosed

Shondoit -- I merged your patch and am rebuilding the Windows Firefox with it. Thanks, and sorry I missed it before, but I didn't deliberately ignore it.

Note: See TracTickets for help on using tickets.