User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/602.3.12 (KHTML, like Gecko) Version/10.0.2 Safari/602.3.12 Steps to reproduce: 1) Enable FlyWeb (dom.flyweb.enabled=true) 2) Open http://mallory.csrf.jp/flyweb.html 3) Push one of the button to activate a FlyWeb service 4) Launch nearby FlyWeb service called 'foo' from FlyWeb menu on Nightly Actual results: 'uiUrl' option of navigator.publishServer allows to set an arbitrary URL including chrome:, file:, and about:. That URL is shown when a victim or other persons in the same local network as the victim try to connect the rogue FlyWeb service. The 3rd button of the above PoC exploits an address bar spoofing vector in about:reader by accessing following URL. about:reader?url=http://firstname.lastname@example.org/ The current implementation of about:reader is not unsafe but it's prone to similar kind of attacks. Expected results: URL with http: or https: schema should be allowed to be set.
Group: firefox-core-security → dom-core-security
Component: Untriaged → DOM: Flyweb
Product: Firefox → Core
Maybe Justin could look into this while Kannan is away? I don't know what layer of code this might be going wrong in.
As I mentioned in an e-mail last year FlyWeb is still in feasibility study and it's not landed on Aurora or Beta. Also it's disabled by default even on Nightly. So I think there would be no risks if the bug is made unhidden. How do ynu think about it and can you open the bug?
Assigning to myself to fix.
Assignee: nobody → jdarcangelo
Status: NEW → ASSIGNED
When NS_NewURI() is passed a `uriString` that is an absolute URI, the `baseURI` parameter is ignored and not prepended (http://i.imgur.com/W4N4mOe.png).
Attachment #8841159 - Flags: review?(jdarcangelo)
Updated to avoid extraneous forward slash when mPath was something like "/controls.html" (would result in a spec of "http://<UUID>//controls.html") as well as adding a necessary forward slash when mPath was missing it e.g. "controls.html".
Attachment #8841243 - Flags: review?(jdarcangelo)
Attachment #8841159 - Flags: review?(jdarcangelo) → review-
Attachment #8841243 - Flags: review?(jdarcangelo) → review+
Please attach that includes appropriate commit information when requesting checkin (name, email, commit message). http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/commits.html#write-detailed-commit-messages https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F
Ron: See Comment 7 above and provide the necessary information to land this patch. Thanks!
Comment on attachment 8841243 [details] [diff] [review] bug1326154-fixed.patch Name: Ron Waisberg Email: email@example.com Commit Message: Fixed improper building of URL by concatenating mHostname with mPath. Previously, the user provided parameter mPath was passed into NS_NewURI() which would ignore and not concatenate the baseURI if mPath was an absolute URI.
seems this need a rebase now since i hit conflicts: patching file dom/flyweb/FlyWebService.cpp Hunk #1 FAILED at 784 1 out of 1 hunks FAILED -- saving rejects to file dom/flyweb/FlyWebService.cpp.rej patch failed, unable to continue (try -v) patch failed, rejects left in working directory errors during apply, please fix and qrefresh bug1326154-fixed.patch could you take a look, thanks!
Attachment #8841159 - Attachment is obsolete: true
Carsten, I am able to apply bug1326154-fixed.patch to inbound without any issues. Also, the affected file (dom/flyweb/FlyWebService.cpp) has not changed for several months so it shouldn't be a rebasing issue. Can you please re-check this? Thanks! (In reply to Carsten Book [:Tomcat] from comment #10) > seems this need a rebase now since i hit conflicts: > > patching file dom/flyweb/FlyWebService.cpp > Hunk #1 FAILED at 784 > 1 out of 1 hunks FAILED -- saving rejects to file > dom/flyweb/FlyWebService.cpp.rej > patch failed, unable to continue (try -v) > patch failed, rejects left in working directory > errors during apply, please fix and qrefresh bug1326154-fixed.patch > > could you take a look, thanks!
hm still hitting the problem ? Ryan could you take a look if you are able to checkin ? Thanks!
Flags: needinfo?(cbook) → needinfo?(ryanvm)
Doesn't apply for me either. Are you working from a git repo? I'm wondering if the patch format is confusing Mercurial for some reason.
Kannan, can you take a look at why Ron's patch isn't applying? I was able to apply to m-c using mq, but the sheriffs have not been able to. I'm not sure what the issue is.
Justin: can you prod this bug along, please?
Updated patch that applies.
Attachment #8841243 - Attachment is obsolete: true
Comment on attachment 8870455 [details] [diff] [review] fix-flyweb-bug-1326154.patch Forwarding prior r+ from justin, since this is the same patch.
Attachment #8870455 - Flags: review+
Comment on attachment 8870455 [details] [diff] [review] fix-flyweb-bug-1326154.patch In the future, you can just set the checkin-needed bug keyword, which plays more nicely with the automated bug marking tools.
Also, please attach a patch that includes proper commit information when requesting checkin.
Attached corrected patch with author and commit message. Carrying over R+.
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/cbae2c2731e4 Fix to prevent FlyWebService from pointing to a URL with a different origin. r=djvj
You need to log in before you can comment on or make changes to this bug.