User Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.28) Gecko/20120306 Firefox/3.6.28 Build ID: 20120306064154 Steps to reproduce: I have successfully built an extension. It tracks browsing URLs, cookies and stores them on secondary storage in private browsing session. I just used normal window.addEventListener("pageshow", OnPageShow, false); & event.originalTarget.location.href to get current URL. I used normal nsICookieManager interface to gather cookies at the end of private mode session. It is just a sample addon created to show that such vulnerability exists. Actual results: It completely violates privacy goals of PM. I found that firefox does not provide browser level privacy mechanism against such attacks. As per AMO, It is said that authors of extensions should respect private mode. I know that in order to publish an add-on it should go through testing at AMO. It is correct that my add-on would get caught at AMO. I need you to consider a case such as I could make temporary addon just to add some functionality to my university annual festival. The addon would report all its users regarding current events & results of competitions. I would make that addon available at my university website. So I would keep my tracking code like mentioned above in that addon which would send all private browsing data of its users to me. In this case AMO does not come into picture to alert students in campus. I only mean to say that there is no browser level detection or prevention mechanism to avoid such attack. *PM=Private Mode Expected results: The whole point of argument is, given a situation browser should be in a position to protect its own privacy. I am nearly finished a patch. I am monitoring calls to all sensitive interfaces [those which are associated with user parameters in private mode] ex. nsICookieManage, nsIFile, nsIProfileManager, nsICacheSession and so on. I used nsIStackFrame to get caller script URLs[modified xpcjsid.cpp]. I have created JS XPCOM[because addon information can be accessed only in JS] component which checks whether script url belongs to any addon. If it belongs to addon then I prompt user such as "XYZ add-on may violate privacy of private mode" & ask permission to disable respective addon. I have stored IDs of such suspicious addons into sqlite. When user requests to enter in private mode[tools->start private mode], just before entering in PM I am disabling all suspicious addons[modified toggleMode() from browser.js and nsPrivateBrowsingService.js]. In order to make disabling effective FF should restart again into PM where suspicious addons are disabled. Finally enabling all blocked addons once user comes out of PM[tools->stop private mode]. There could be many other ways to steal user specific information in PM such as storing on RAM or in Ctype but i tried to prevent attack based on standard interfaces. I know it is hard to digest for most of you but according to my project, all those add-ons who access sensitive interfaces should not be allowed in private mode or browser should inform user regarding side effect they might cause. All of this should happen at browser level. All suggestions to make my solution efficient are most welcomed.

There are still some commented code segment which I would remove while next version of patch. Recovery of previously opened tabs is yet to do. Let me know how this works. it catches "chatzilla" as this addon does file operation in private mode. So you can test on it. It also detect several other addons.

Way to test: 1. open PB: start any addon [here try with chatzilla] it will detect it as it performs file handling in PM. 2. Set the check box & OK. 3. start FF again & start private mode. It restarts while this process it also disables all blacklisted addons.[here ex. chatzilla]. One small issue: It opens two PM windows. I am trying to avoid one of them. 4. when tools->stop private mode. It again restarts & enables all blocked addons[here chatzilla].

Addons can do anything. Of course you can write a spyware/malware addon. Addons are simply additional software running in the browser and have access to all the capabilities of the browser. This is a given, and not something that can be changed. The only way to avoid this would be to require all addons to only use the SDK and ban all normal addons, which would ban the majority of addons. This isn't going to happen. The bottom line, as has been stated many times elsewhere: Don't install addons from sources you don't trust. Don't think of installing addons as being different from installing other software. Mozilla also maintains a list of malware addons that are blocklisted for all Firefox users.

I believe that Ameya's idea is actually interesting. The patch serves as a sort of prototype spyware/malware detector. While I am not sure that we want to use the patch as it is now, it could serve as a base for further exploratory work.

For any automated detection to work it would really need to be done at addon install time, not run time. Any malware addon that gets installed would just disable/override runtime checks. An automatic evaluation of an addon's security and privacy prior to install would be awesome, but quite a lot of work.

As to non-malware addons that simply have bad practices, I think your base idea has merit, but including such a thing as proposed in comment 0 in Firefox is just not a practical idea. Even if we were to do as you suggest, there are so many ways an addon could violate your privacy that such a system wouldn't be, by itself at least, reliable to detect when an addon does something bad. If you can get a fully working addon to detect addons that have bad practices during private browsing, that would be a wonderful thing not just for people to test addons not from AMO, but also to give to AMO editors to make things easier for them to review addons.

(In reply to Dave Garrett from comment #3) > Addons can do anything. Of course you can write a spyware/malware addon. > Addons are simply additional software running in the browser and have access > to all the capabilities of the browser. This is a given, and not something > that can be changed. The only way to avoid this would be to require all > addons to only use the SDK and ban all normal addons, which would ban the > majority of addons. This isn't going to happen. Correct, It might affect to lot of add-ons but when we propose something like private mode. We should always to try justify its primary goal whatever may be the cost. After all user is going to ultimate decision maker whether he/she wants to disable such addons or not. > > The bottom line, as has been stated many times elsewhere: Don't install > addons from sources you don't trust. Don't think of installing addons as > being different from installing other software. Basic logic to all of this starts here, what if someone uses un-trusted addons during private mode?? Does browser have enough filtering mechanism to alert user..? Apart from saying its un-trusted software. I think it is fair to ask users whether they want to run suspicious addons in private mode or not..? Knowing the power of addons, they need to be put under some restriction at least during private mode. > > Mozilla also maintains a list of malware addons that are blocklisted for all > Firefox users. Yes, but I found a strange statement under Mozilla Public License [http://www.mozilla.org/MPL/1.1/ at 7.DISCLAIMER OF WARRANTY]which says “The entire risk as to the quality and performance of the covered code is with you. Should any covered code prove defective in any respect, you (not the initial developer or any other contributor) assume the cost of any necessary servicing, repair or correction”. This means above license is also an indication/notification. I think my approach to alert users would also work same way even when Addon is not published at AMO. The whole point is some privacy prevention mechanism should be added at browser level.

(In reply to Dave Garrett from comment #5) > For any automated detection to work it would really need to be done at addon > install time, not run time. To see whether addon really accesses sensitive interfaces in PM or not, it is required to follow them at runtime. > Any malware addon that gets installed would just > disable/override runtime checks. But I tried to implement monitoring at XPConnect level [In xpcjsid.cpp]. Could addon override this checking?? Correct me if I am wrong.. > An automatic evaluation of an addon's security and privacy prior to install would be awesome, but quite a lot of > work.

(In reply to Dave Garrett from comment #6) > As to non-malware addons that simply have bad practices, I think your base > idea has merit, but including such a thing as proposed in comment 0 in > Firefox is just not a practical idea. Even if we were to do as you suggest, > there are so many ways an addon could violate your privacy that such a > system wouldn't be, by itself at least, reliable to detect when an addon > does something bad. Yes Correct, there are several other ways to violate privacy. I tried to detect & prevent some orthodox manner which involves using interfaces that allows to track and then could be stored on secondary storage. I mean storing state of PM on RAM or in Ctype looks un-orthodox way. > > If you can get a fully working addon to detect addons that have bad > practices during private browsing, that would be a wonderful thing not just > for people to test addons not from AMO, but also to give to AMO editors to > make things easier for them to review addons. Actually I had the idea of making an addon to monitor other addon at first place but it does not look ethical. I mean both would be addons & why would user listen to one addon which says other addon is suspicious as either of them can not be trusted completely. Yes, It could make a job of AMO editors easy but here my objective was to improve strength of browser. I need your suggestion to make this concept more efficient. Thank you for replying to my bug. Waiting for more suggestions.

(In reply to Dave Garrett from comment #5) > For any automated detection to work it would really need to be done at addon > install time, not run time. Any malware addon that gets installed would just > disable/override runtime checks. An automatic evaluation of an addon's > security and privacy prior to install would be awesome, but quite a lot of > work. Yeah, JavaScript static analysis! Sounds so very likely :) (In reply to Dave Garrett from comment #6) > As to non-malware addons that simply have bad practices, I think your base > idea has merit, but including such a thing as proposed in comment 0 in > Firefox is just not a practical idea. Even if we were to do as you suggest, > there are so many ways an addon could violate your privacy that such a > system wouldn't be, by itself at least, reliable to detect when an addon > does something bad. > > If you can get a fully working addon to detect addons that have bad > practices during private browsing, that would be a wonderful thing not just > for people to test addons not from AMO, but also to give to AMO editors to > make things easier for them to review addons. Sounds like a plan. Perhaps Ameya could consider repurposing his patch into a testing tool.

(In reply to David Rajchenbach Teller [:Yoric] from comment #10) > Yeah, JavaScript static analysis! Sounds so very likely :) The AMO addon validator already does quite a lot of static analysis. It might be possible to augment it to check for bad PB usage too (my guess is it may already have something), but I'm not sure how well it's really set up for that. Yeah, doing this sort of thing in the Addon Manager itself would be a tall order, but it would be awesome. (In reply to Ameya from comment #7) > Basic logic to all of this starts here, what if someone uses un-trusted > addons during private mode?? If it's malware we're talking about, as you start comment 0 with, the user is already screwed. That's why I said such things need to be caught at install time (or more precisely, tested directly prior to install). If it's just poorly written and violates PB mode, that's another story. Trying in vain to automatically have the browser detect and cope with such a situation to some degree isn't the best route. Creating a way to better detect the problems to report them and get them fixed is a much better idea. Particularly bad addons should be blocklisted. Even if you could come up with a polished in-browser detection mechanism, I don't see adding such a thing as winning a cost-benefit analysis. It'd add a fair bit user-facing messages that would likely either be confusing or ignored and result in more complexity added to things that won't get enough of a help from it. This concept really does sound better as a separate tool. > I think it is fair to ask users whether they > want to run suspicious addons in private mode or not..? From a purely logistical standpoint: how? Most addons still aren't restartless. You're not going to be able to disable and re-enable them at runtime. Your only route would be to creating some sort of firewall that filters the addons' bad actions, which sounds like an even more messy endeavour. > Yes, but I found a strange statement under Mozilla Public License > [http://www.mozilla.org/MPL/1.1/ at 7.DISCLAIMER OF WARRANTY]which says “The > entire risk as to the quality and performance of the covered code is with > you. Should any covered code prove defective in any respect, you (not the > initial developer or any other contributor) assume the cost of any > necessary servicing, repair or correction”. This is just legal boilerplate that's in every software license. It just means that you can't sue Mozilla or any Mozilla developers for a Firefox bug. (In reply to Ameya from comment #8) > Could addon override... Yes. An addon could be written to override anything.

(In reply to Dave Garrett from comment #11) > From a purely logistical standpoint: how? Most addons still aren't > restartless. You're not going to be able to disable and re-enable them at > runtime. Your only route would be to creating some sort of firewall that > filters the addons' bad actions, which sounds like an even more messy > endeavour. To answer my own question: Your code uses nsIAppStartup to restart the application. That isn't going to be doable once window-level private browsing lands and people can have both private and regular windows open at the same time. Even with restartless addons you'd be disabling it application-wide.

> Sounds like a plan. Perhaps Ameya could consider repurposing his patch into > a testing tool. Ok fine. I will try convert it into testing tool. Could Dave Garrett, yoric elaborate this idea more ? how exactly this application/addon would look..?? I mean what would be the exact requirements of this testing tool...?

The general procedure I was thinking about would be as follows: 0) User would first create a new Firefox profile (as is already done for editor reviews) 1) Install addon to test and PB testing addon 2) The user would open private browsing mode 3) The user would then go through the various features of the addon being tested 4) When the testing is done, the user would click on a menu item to load a report 5) The PB checker addon would load up a tab with a list of all bad PB practices it logged So this PB checker addon would keep a log of all bad PB practices as it finds them and then the user would open up a menu to view them all in detail. Basically the same sort of thing you're doing now, except a detailed report at the end instead of a prompt for each violation. Even if a more elaborate tool or other system is to be designed, having this put together as a fully functional addon could be useful.

First of all, thanks a lot Ameya for the amazing work so far! This is really impressive. :-) So, first, let me share some background on this. It's impossible to use any sort of static analysis to detect this kind of problem in an add-on without running it, since by definition you cannot know whether a piece of code will run in private mode or not (as the private mode might be toggled outside of the add-on.) Also, the goal here is *not* to catch all the cases where an add-on may hurt the privacy of our users, as that is clearly impossible to achieve. In other words, we're not trying to defend against malware, we're trying to defend against add-ons which are badly written and do not respect the private browsing mode. Now on to the possible ways in which we can use this idea. 1. We can build a custom version of Firefox for AMO editors to use to test add-ons for violations of private browsing mode. This is definitely needed, as currently we require them to look for this kind of pattern but we don't provide any tools for them. This is clearly a big win for add-ons hosted on AMO. But that clearly doesn't include all add-ons. 2. We can bake this into Firefox itself, and include it in the browser that we ship by default. That of course means that the patch needs to meet a higher quality bar. Most importantly, the user interactions need to be polished. For example, having to restart the browser in the middle of the browsing session is not ideal. Also we should figure out if we even want Firefox to automatically disable such add-ons, or just hint the user about what's happening, etc. Or, we may decide that we want to go down this route only to get reports from users on what add-ons are affected by these problems, and try to get in touch with their authors etc. 3. We need to figure out a path forward to the per-window private browsing mode which is currently under development. In per-window private browsing mode, the global private mode flag does not exist any more, and one needs to figure out which window a given request is coming from. I need to think more about items 2 and 3... I'll spend some time giving this more thought, and I'll try to comment again tomorrow. It would be great to hear what Josh thinks here as well.

@Ameya As a general guideline in the Mozilla project, I suggest the following steps: - Divide the bug into many small tasks that will require only a short patch each; - Get the first patch reviewed as soon as possible, so that you can learn how to improve it – and use that knowledge for the next patch, etc. - Get to the second patch once there seems to be some agreement on the first patch, etc.

Can we please let this bug track the general project? We can file individual bugs for each of the items I mentioned in comment 15 if needed. No need to morph this bug into smaller subtasks.

ok

(In reply to Ehsan Akhgari [:ehsan] from comment #15) > 1. We can build a custom version of Firefox for AMO editors to use to test > add-ons for violations of private browsing mode. This is definitely needed, > as currently we require them to look for this kind of pattern but we don't > provide any tools for them. This is clearly a big win for add-ons hosted on > AMO. But that clearly doesn't include all add-ons. I found https://github.com/mozilla/amo-validator from #amo on IRC. Could it be one of the place where our mentioned approach is useful ?

(In reply to Ameya from comment #19) > (In reply to Ehsan Akhgari [:ehsan] from comment #15) > > 1. We can build a custom version of Firefox for AMO editors to use to test > > add-ons for violations of private browsing mode. This is definitely needed, > > as currently we require them to look for this kind of pattern but we don't > > provide any tools for them. This is clearly a big win for add-ons hosted on > > AMO. But that clearly doesn't include all add-ons. > > I found https://github.com/mozilla/amo-validator from #amo on IRC. Could it > be one of the place where our mentioned approach is useful ? Not sure, that's a question for Jorge.

The AMO validator is used for static code analysis, not feature testing. As for having a custom Firefox version, I don't think that's such a good idea. Our testing should be as close to real-world usage as possible, and I wouldn't want to place any hurdles for reviewers ("you need to install this special version that is probably out of date"). We try to check for PBM violations; it's part of our policy. However, it's not the most important, so oversights can happen. The same mistakes can happen in other areas, so it's just something we have to live with. We have other mechanisms to deal with problems after the fact. Not to say that it wouldn't be awesome to have better testing tools ;)

So how do you propose this testing tool should be developed?

As an add-on, of course :)

(In reply to Jorge Villalobos [:jorgev] from comment #23) > As an add-on, of course :) That's going to be tricky, as this needs modifications to the deep guts of xpconnect...

Most editors currently use an add-on of mine called Extension Test (https://addons.mozilla.org/addon/extension-test) to test for runtime issues that we can't find with static validation. If there's willingness among core developers to support us in these kinds of tests, it would be more helpful, I think, to provide an interface that testing add-ons can hook into for these kinds of data. And if that's going to happen, it would be really helpful to have a discussion about what data would be most useful to is. I'm not entirely sure how much we'd be able to act upon this kind of information, even at runtime. Our restrictions on add-ons in PBM are basically that they can't write data relating to browsing history to disk while in PBM. They can still send it to remote servers, those servers can even use it for tracking purposes. I'm not in the camp that approves of those policies, but that's where we are. Even that aside, I'm not sure how useful it would be for us to know what data are being accessed while in PBM. Add-ons are more or less supposed to behave as normal, except that they're not supposed to be writing data to disk, so knowing what data they are reading during PBM is not especially useful. I'd expect them to access roughly the same data as they would outside of PBM. Knowing what data they write might be more useful (a log of database writes might be, while infeasible, somewhat helpful). On the other hand, the issue I'm currently struggling with, tracking XMLHttpRequests opened by chrome code to check for remote, synchronous requests and other common issues without breaking certain strangely written extensions, and every chrome consumer of XHRs in 15, without having to write a binary component would be much more useful. Tracking the creation of Sandbox and module globals would also be handy.

Note the parallel discussion on bug 763904.

(In reply to Kris Maglione [:kmag] from comment #25) > I'm not entirely sure how much we'd be able to act upon this kind of > information, even at runtime. Our restrictions on add-ons in PBM are > basically that they can't write data relating to browsing history to disk > while in PBM. > Add-ons are more or less supposed to > behave as normal, except that they're not supposed to be writing data to > disk, so knowing what data they are reading during PBM is not especially > useful. I'd expect them to access roughly the same data as they would > outside of PBM. Knowing what data they write might be more useful (a log of > database writes might be, while infeasible, somewhat helpful). But it is not limited only to writing browsing history. On broader sense let's see ChatZilla, no offence, it's a great extension.. but say, for whole day I used PBM & used chatzilla a lot. ChatZilla is going to log my chats on disk irrespective of browsing mode.. So tomorrow anyone else is going to find that I chatted for long period. It is also kind of privacy leak. There are several other small issues like some download-related extensions keep list of files to be downloaded on disk till they don't complete download [without considering PBM]. So, information about the files you kept for downloading in PBM will remain on disk if you leave PBM & still download is not complete. which is also a leak. Its fine in non-PBM but PB. Hence I would suggest instead of concentrating on what content[ex. data or log] it is writing, it is fine/[enough to] look at what resource[ex. file handling] it is accessing, because when some add-on uses sensitive resource in PB then there is a high probability that intentionally or unintentionally it may leave some trace about user's activity outside private mode.

There are a few separate issues here. 1) From a policy perspective, it's not clear what add-on like Chatzilla should do in PBM. PBM relates specifically to browsing, and the decision on what it should do in relation to logging and backlogs in private mode would have to be made on a case by case basis, and would fall to Jorge. 2) In the case of downloads, the download log is arguably browsing history and should be removed, but downloaded files should not. So add-ons should be held accountable for preserving it. 3) Given the above, it's not clear how monitoring what data add-ons access would be terribly useful. In particular, if we want to make sure that add-ons which are accessing certain types of data are taking account of PBM, it would make more sense to create static validator warnings when the data is accessed saying that PBM must be accounted for than it would to do runtime checks, since I'd expect them to access the data whether in PBM or out. Beyond that, it might be helpful to add another warning if the add-on doesn't seem to be doing any PBM checks when it accesses those data. But I'm not sure how helpful, and I suspect not terribly much. For IO, however, I agree it would be very useful to keep track of what files, databases, and storage resources an add-on accesses. I tried once to do this from an extension, and it turned out to be entirely infeasible. So if we can create an interface for this, that would be a massively useful, not just for PBM checks but for broader security checks as well.

Once again hello friends... I am very sorry that I was away for many days but now i need to start work on it once again. Could you people suggest any further plan?

(In reply to Kris Maglione [:kmag] from comment #28) > In particular, if we want to make sure that add-ons which are accessing > certain types of data are taking account of PBM, it would make more sense to > create static validator warnings when the data is accessed saying that PBM > must be accounted for than it would to do runtime checks, since I'd expect > them to access the data whether in PBM or out. Beyond that, it might be > helpful to add another warning if the add-on doesn't seem to be doing any > PBM checks when it accesses those data. But I'm not sure how helpful, and I > suspect not terribly much. > > For IO, however, I agree it would be very useful to keep track of what > files, databases, and storage resources an add-on accesses. I tried once to > do this from an extension, and it turned out to be entirely infeasible. So > if we can create an interface for this, that would be a massively useful, > not just for PBM checks but for broader security checks as well. As per my knowledge, we could check which data[user specific] is being accessed by extensions in PBM. It seems difficult to find what does extension do with that data because in order to find that we need to maintain tracking tool on xtension once it accesses any sensitive data. Is it possible to do ? It looks super complex. If we could track somehow then we can precisely monitor & restrain addon in case of violation. Any idea on how to keep track on extensions??

(In reply to Kris Maglione [:kmag] from comment #28) > For IO, however, I agree it would be very useful to keep track of what > files, databases, and storage resources an add-on accesses. I tried once to > do this from an extension, and it turned out to be entirely infeasible. So > if we can create an interface for this, that would be a massively useful, > not just for PBM checks but for broader security checks as well. Your view seems useful application of monitoring concept. Could you elaborate it so that i could modify my patch as per that. Could you explain more what you tried to do with extension which was infeasible ? As i am modifying source, problem faced by your extension-form could be solved by my patch...

Ameya emailed me about applying this concept to look for mainthread IO. As I understand it, the patch looks at who is using certain GetService calls(cleverly easy way to bin badness) and marks those addons. I think we should be able to accomplish the same thing with the new js debugger api by setting programmatic breakpoints, so there is no need to modify xpconnect. I agree with others that the concept applied here is usually more powerful than static analysis. On the other hand if an addon uses clever timing to avoid doing bad things during a short session typical of our AMO testing process, dynamic testing wont catch anything. IMHO the best use of this would be to develop this as an addon using the js debugger API to provide it as an extension to the more paranoid of us to be enabled during private browsing. In order to use this approach to track IO it would be more useful to blacklist specific api calls to cover things like xmlhttprequest, sync storage apis(but not async ones).

Unfortunately, Debugger has some restrictions that prevent it from being useful here. 1) Debugger can't set breakpoints that trigger when native functions are called. This is a disappointing restriction. Filed as bug 791846. 2) Debugger generates stack traces that only contain frames running debuggee code. If you haven't added the global of the code the frame is running as a debuggee, the stack just omits it. (This is how we distinguish chrome from content.) Now, we could simply make everything a debuggee in order to get the stack trace, but... 3) We can only add a global as a debuggee if there are no frames currently executing in that global. Basically, a live frame can't go from being a non-debuggee to being a debuggee. (This is bug 716647.) So once you've got frames on the stack, if you didn't say up front that you wanted to debug them, you can't get a backtrace through them. Now, you could just add everything as a debuggee up front, except... 4) Adding a global as a debuggee reduces the performance of the code scoped to that global. Debuggee-ness disables IonMonkey, and inhibits optimizations in JaegerMonkey as well. It does so to make features like Debugger.Frame.prototype.eval work, that evaluate code in the scope of a given frame. However, there are plenty of debugger applications (like the bug that this comment belongs to) that don't need to do anything fancy --- they just want a backtrace. Debugger gives you powerful features, and charges you a high price for them, whether you use those features or not. (This is bug 783428.) I don't know if it's sufficient, but could you instead use '(new Error).stack' to find the filenames of the callers?

(In reply to Jim Blandy :jimb from comment #33) > > I don't know if it's sufficient, but could you instead use '(new > Error).stack' to find the filenames of the callers? Then the problem is needing to wrap natives(ie GetService) remains. Do you have clever suggestions on how to do that?