Consider this code from ServiceWorkerManager.cpp: // Step 8 "Queue a task..." for updatefound. nsCOMPtr<nsIRunnable> upr = NS_NewRunnableMethodWithArg<ServiceWorkerRegistrationInfo*>( swm, &ServiceWorkerManager::FireUpdateFoundOnServiceWorkerRegistrations, mRegistration); NS_DispatchToMainThread(upr); This dispatches a runnable without holding a strong ref to the mRegistration argument.

Marking sec-high, because I don't think this is easily exploitable.

Fixed another case I found as well.

Passes tests locally.

Comment on attachment 8696031 [details] [diff] [review] Hold a strong ref in service worker NS_NewRunnableMethodWithArg uses. r=ehsan [Security approval request comment] How easily could an exploit be constructed based on the patch? I think difficult. Its hard to directly control the life cycle of these objects from script. Even if this was possible, it would then need to force the race to fail. Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem? Well, its obviously a potential UAF, but again I don't think its obviously exploitable. Which older supported branches are affected by this flaw? I believe we should fix this in 44. Its probably been in the service worker code the entire time, but its disabled behind a pref on earlier branches. If not all supported branches, which bug introduced the flaw? Probably there from the first service worker manager impl. Again, disabled on earlier branches. Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be? This patch should uplift cleanly to aurora 44. How likely is this patch to cause regressions; how much testing does it need? No risk. Its simply holding a strong ref where the code expects the object to stay alive. Tested locally using existing mochitests and wpt tests.

Comment on attachment 8696031 [details] [diff] [review] Hold a strong ref in service worker NS_NewRunnableMethodWithArg uses. r=ehsan Approval Request Comment [Feature/regressing bug #]: Service workers [User impact if declined]: Probably minimal, since this seems hard to exploit. It would be nice to close the UAF race, though, since we are shipping SW on 44. [Describe test coverage new/current, TreeHerder]: Existing mochitests and wpt tests. [Risks and why]: Minimal. Service workers only. [String/UUID change made/needed]: None.

Comment on attachment 8696031 [details] [diff] [review] Hold a strong ref in service worker NS_NewRunnableMethodWithArg uses. r=ehsan Approvals given.

m-i: remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/6c582cc65dc9 m-a: remote: https://hg.mozilla.org/releases/mozilla-aurora/rev/eeaa77b2a1e0