Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HUD cant be used remotely unless browser cache cleared #204

Open
psiinon opened this issue Oct 8, 2018 · 16 comments
Open

HUD cant be used remotely unless browser cache cleared #204

psiinon opened this issue Oct 8, 2018 · 16 comments
Labels

Comments

@psiinon
Copy link
Member

psiinon commented Oct 8, 2018

I've found that I need to manually clear the (Firefox) browser cache every time before the HUD will work with a remote browser.
To reproduce:

  1. Start ZAP on one machine
  2. Configure ZAP (and machine) to accept remote requests
  3. Configure browser on 2nd machine to proxy through ZAP
  4. Proxy to target, HUD will probably work
  5. Close browser (or just the tab), reopen it and open the target again
  6. HUD will load but none of the popups work

Errors logging in the browser:

ERROR errorHandler: [object ErrorEvent]: {"isTrusted":true}

Clearing the cache while the tab is still open results in a load of errors like:

errorHandler: InvalidStateError: A mutation operation was attempted on a database that did not allow mutations. z https://zap//zapCallBackUrl/957943243191398600?name=libraries/localforage.min.js 7:941: {}

No ZAP side errors are being logged.

@dscrobonia any thoughts??

@psiinon psiinon added the bug label Oct 8, 2018
psiinon added a commit that referenced this issue Oct 8, 2018
Remove Referrer-Policy header - Fixes #197
Set cache control to no-cache, no-store - prevents the browser from
reusing a cached version of a page that has old callback urls in.

Note that this does not fix problems with remote access (#204)
@dvas0004
Copy link
Contributor

Hey @psiinon

I've been trying to reproduce this but I'm running into another issue which I think may be browser related and wanted to see if you came across it. I'm following your steps above, and covered steps 1-4. However on step 4, looking at the console I'm seeing a bunch of errors like so:

image

Those CSP errors are being thrown by callback URLs such as:

https://zap//zapCallBackUrl/6223563515382454344?name=drawer.html
https://zap//zapCallBackUrl/6223563515382454344?name=growlerAlerts.html

Weird thing is, checking the network requests I see the CSP policy set correctly:

image

For clarity the CSP in the screenshot is:
default-src 'none'; script-src 'self' 'unsafe-eval'; connect-src https://zap wss://zap; frame-src 'self'; img-src 'self' data:; font-src 'self' data:; style-src 'self' 'unsafe-inline'

This of course is showing the HUD but nothing works...
Another point, starting FF from within ZAP i.e. from the "Launch Browser" button in the quick start, doesn't produce these issues

Did you run across anything like this?

@psiinon
Copy link
Member Author

psiinon commented Oct 10, 2018

Ah, so I nearly always run ZAP using Browser Launch.
I definitely need to get some unit tests working so that we can test these things automatically.
I'll also have a manual play asap as well.

@dvas0004
Copy link
Contributor

dvas0004 commented Oct 10, 2018

To be honest so do I, this was the first time I did the "manual launch" since my second machine didn't have ZAP. Ok so i'll try reproduce this cache issue using browser launch because the CSP looks like a seperate issue

@thc202
Copy link
Member

thc202 commented Oct 10, 2018

Which site were you accessing? It might be related to #42, although the CSP of the target should be removed by default, did you change any HUD options?

@dscrobonia
Copy link
Contributor

dscrobonia commented Oct 10, 2018

@dvas0004 the CSP issue should be able to be mitigated by selecting the HUD options via Tools -> Options and then choosing the Development options -> allow unsafe eval I actually ran into this problem last week when playing on Chrome and it took me way too long to remember ha.

image

@dvas0004
Copy link
Contributor

The site I was accessing is https://example.com, and my HUD options are exactly as shown by @dscrobonia because I had to enable "Allow unsafe evals" while doing VueJS development.

@dscrobonia
Copy link
Contributor

@psiinon what is a remote browsing session? I am not familiar.

As per your error message I might guess that clearing the cache also clears out data about the domain in localstorage and indexeddb. Which means that when the HUD tries to read from indexeddb it won't find anything.
I also foutnd this bug thead about that error message and it looks like indexeddb doesn't work in private browsing mode for Firefox: peer-base/peer-pad#36 :// which is also abummer.

@dscrobonia
Copy link
Contributor

@dvas0004 I'll have to take another look at that then, because I almost alwasy run the HUD manually and haven't seen that pop up before on FF. :/

@dvas0004
Copy link
Contributor

dvas0004 commented Oct 10, 2018

Just tried same setup (manually launched and proxy set up) with Chrome and it works - no changes to ZAP... suspecting an FF issue...

PS this refers to the CSP issue, not the cache issue, i'm spinning off the CSP issue to a separate thread ( issue #211 )

@psiinon
Copy link
Member Author

psiinon commented Oct 10, 2018

@dscrobonia by remote browsing I mean ZAP running on one machine and your browser (and therefore the HUD) running on another. You need to configure ZAP to allow it by setting the local proxy addr to 0.0.0.0 or a specific remotely accessible addr (from memory). And any firewall changes of course

@dvas0004
Copy link
Contributor

Ok this is a bit of a tricky one, that I think is actually related to #199 . Let me step through this a bit:

Aspects of the problem
I've managed to reproduce this exactly as @psiinon says, but there's another subtle twist to the bug. If you open two tabs side by side and visit the same target, you get the same behavior @psiinon describes. This is an important detail and hence my suspicion that it is related to #199

Context
Let's assume we're using the "Site Alerts" tool. In a normal scenario, whenever we click on a tool icon, VueJS fires of an "onClick" event. In our event handler for this event, we translate this to a serviceworker event and then expect that the serviceworker has it's own event listeners registered by the various tools we import. Among these tools are various alerts for example "Site Alerts Low", which when they receive this "buttonClicked" serviceworker event, call a function to display the alerts.

Troubleshooting
I inserted a bunch of "console.error" statements throughout the code to observe the above (unfortunately for me the "debugger" statement just never fired). Reproducing as @psiinon says, I could see that the VueJS "onClick" event was correctly triggered, but on the serviceworker side, only one event listener was being fired, so none of the tool event listeners were being fired

Some more console statement later and we note that for some weird reason, whenever firefox opens a new tab: it clears the new tab's serviceworker global scope (the self object) and doesn't re-run the importScripts. So now we have the weird situation where if the serviceworker Event Listeners were added in the main script, they're fine, but if we added any Event Listeners using "self.addEventListener", those are gone.... So there are no event listeners to react when we click on HUD buttons in this new tab.

Mitigations/Workarounds
I personally think this is a FF issue - my personal opinion is that "self" should be preserved across tabs (Chrome seems to do this). But in any case, the workaround i've come up with that every time the main service worker file receives an event message, it re-imports the scripts, like so:

const onMessage = event => {
	if (!isFromTrustedOrigin(event)) {
		return;
	}

	var message = event.data;
       // ADDED THE CODE BELOW
	for (var toolScriptCounter = 0; toolScriptCounter < toolScripts.length; toolScriptCounter++){              
             var script = toolScripts[toolScriptCounter];
             importScripts(script);
       }
[...]

This now makes the plugins work without needing to refresh the cache, but now we start running into the errors which @psiinon spoke of when he cleared the cache. These errors are basically due to the fact that the serviceworker scope has been cleared and none of the functions the importScripts defined are being re-run, so a lot of operations are returning "undefined", leading to those errors...

I'm not sure what the silver bullet solution here is. If all the above is actually correct and this is not an FF bug, this means that all our scripts would need to live in one large serviceworker.js file - a very clunky solution IMHO. Nevertheless I did this and created a Frankenstein file made up of the various scripts in one place, which you can find attached. If you copy/paste the contents of this file to replace the contents of serviceworker.js you'll see that most of the issues are actually resolved - @psiinon could you confirm this please?

special_sw.zip

I need a coffee....

@psiinon
Copy link
Member Author

psiinon commented Oct 11, 2018

@dvas0004 glad you're looking into this!
One quick point - we know the HUD doesnt work for multiple tabs - thats #119 (which @dscrobonia is working on), and we also know theres a bug in Firefox re having a serviceworker per tab #199 - does that change your understanding of this problem?

@dvas0004
Copy link
Contributor

In fact I was looking for #119 because I was sure you guys must have come across the subsequent issues. I think that this issue is in fact another part of that bug....

In #199 you guys highlighted that the dialog only appears in the latest tab opened. But at least in my case what I observed what that only the buttons on the first tab would trigger a dialog on the second tab. But if you close the first tab as per your reproduction steps above it would appear as if nothing is working. This is what I was seeing:

hud_without_frk

None of the plugins worked after closing the first tab. By combining all the scripts into one, we get to this:

hud_with_frk

The dialogues now popup as expected but we run into all the issues you discuss in #119

But in #199 we noted that there are seperate SWs for each tab. In my case it was not the case... I had one SW for multiple tabs:

hud_sw

But, the service worker acted as if it was stateless as @dscrobonia mentions, though in addition it doesn't re-run the importScripts statements - which lead to this particular issue

Maybe that helps a bit?

@dscrobonia
Copy link
Contributor

Ah.. @dvas0004 There is still "one service worker" but if you add a few lines to generate a random number at the top of the serviceworker file, save it to a variable, and then log that variable out everytime the serviceworker receives a message you should see that with multiple tabs in firefox there are actually two different random numbers being logged in the two tabs (indicating that there are multiple processes running) where as in chrome the number is the same (indicating the same process is running)

Does that make sense? Let me know if it doesn't

@dvas0004
Copy link
Contributor

dvas0004 commented Oct 17, 2018 via email

@dvas0004
Copy link
Contributor

Actually there is a way around this - and it's been staring me in the face the entire time. As you say @dscrobonia you'll get two random numbers being logged unless you open the FF menu > Web Developer > Click on "Opt out of multiple content processes"

screenshot from 2018-10-17 09-52-20

The browser will restart, and visit the above to make sure the setting persisted. Then re-run your random number test and it's resolved. In my tests it also resolves this particular issue we're discussing here (@psiinon can you confirm please? though obviously it doesnt solve the dialogue only shows on latest tab)

This is obviously a workaround but some further research shows that @psiinon was dead on the money in his comment in issue #199 :

https://stackoverflow.com/questions/47414543/firefox-quantum-sharedworker-not-work

If you look in the above SO question, in the comment, there's a link to the same bug that @psiinon mentioned. In fact I think you can close #199 and this issue (if @psiinon confirms it works) with a note of this workaround because I doubt there's anything we should do considering the mozilla team are working to resolve this. I would imagine that whatever we code to workaround this issue would probably break as soon as a fix is released?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

4 participants