User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.101 Safari/537.36 Steps to reproduce: 1. enable dom.moduleScripts.enabled pref 2. Go to https://test.shhnjk.com/test.html 3 Reload the page Actual results: Crash. https://crash-stats.mozilla.com/report/index/f974d23e-b867-4b19-9741-3db9e0170901 Expected results: should not crash. Tested on latest Nightly on Windows 10.
:jonco, seems the pref mentioned in comment #0 was added in a patch you wrote, so I'm hoping you know who can look at this? :-) Not entirely sure this needs to be sec-sensitive, crash looks like a near-null read to me (but please check, not an expert).
Group: firefox-core-security → core-security
Product: Firefox → Core
Assignee: nobody → jcoppeard
This is not sec-sensitive as JS modules are disabled by default.
The problem is that ModuleLoadRequest::ModuleErrored() asserts that the module script is null or errored after the status of all a module's dependencies are checked, but in ScriptLoader::StartFetchingModuleAndDependencies() we can hit an error loading a dependency before we we add that dependency to the list in the load request and hence it appears that the module's dependencies all loaded OK. The fix is just to add dependent load requests to the list before we try to load them.
Attachment #8904246 - Flags: review?(bkelly)
Comment on attachment 8904246 [details] [diff] [review] bug1395896-mixed-content-import Review of attachment 8904246 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for adding a test!
Attachment #8904246 - Flags: review?(bkelly) → review+
Daniel, does this really need to be private given the feature is disabled by default?
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
status-firefox57: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
status-firefox55: --- → disabled
status-firefox56: --- → disabled
status-firefox-esr52: --- → disabled
(In reply to Ben Kelly [:bkelly] from comment #5) > Daniel, does this really need to be private given the feature is disabled by default? Since it's fixed, no. In general if not fixed it would depend on how confident you are we'd fix a given security bug before enabling the feature.
You need to log in before you can comment on or make changes to this bug.