Status

()

defect
P2
normal
VERIFIED FIXED
Last year
Last year

People

(Reporter: vdnpp, Assigned: valentin.gosu)

Tracking

({crash, csectype-dos, testcase})

59 Branch
mozilla62
Points:
---

Firefox Tracking Flags

(firefox-esr52 wontfix, firefox-esr60 wontfix, firefox60 wontfix, firefox61 wontfix, firefox62 verified, firefox63 verified)

Details

(Whiteboard: [necko-triaged][sg:dos], crash signature)

Attachments

(2 attachments, 1 obsolete attachment)

Posted file screenshot.tar.gz
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0
Build ID: 20180323154952

Steps to reproduce:

OS: Debian 9
Browser: 59.0.2 64bit and 52.7.4 ESR 64bit

1. Open in tab ftp://cyberghost.pro:80/pro.php (any exec file which not on server)
2. After a while all the tabs crashed in 52.7.4 ESR version and only active tabs crashed in 59.0.2 version

For example, may be use redirect for reproduction - https://cyberghost.pro/tor/r.php






Actual results:

After try open FTP link on non default port and with the indication of certain types of files such as php, doc, xls  (any exec type file) there is a crashed browser.


Expected results:

Browser must inform you that ftp is not used at this address.
This reproduces for me on Linux, but not Win10. Valentin, can you please take a look? It's also unclear to me why this was filed as a security-sensitive bug?
Group: core-security → network-core-security
Crash Signature: [@ IPCError-content | PFTPChannel::Msg_FlushedForDiversion Route error: message sent to unknown actor ID ]
Flags: needinfo?(vdnpp)
Flags: needinfo?(valentin.gosu)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #1)
> It's also unclear to me why this was filed as a
> security-sensitive bug?

That's been my suggestion as I did not have time to look into this yesterday and was not sure about the severity of this issue. It seems we hit

Assertion failure: ((bool)(__builtin_expect(!!(!NS_FAILED_impl(rv)), 1))) || rv == NS_ERROR_NOT_AVAILABLE, at /var/tmp/build/firefox-deaa82b4f8ab/netwerk/protocol/ftp/FTPChannelParent.cpp:646
Assignee: nobody → valentin.gosu
Flags: needinfo?(valentin.gosu)
Whiteboard: [necko-triaged]
Flags: needinfo?(vdnpp)
Good news: with the latest nightly this doesn't crash anymore. (bug 1436311)
Bad news: infinite loop :)
The bug is indeed caused by the tcp connection not sending back any data, and just being closed right away.
So we get something like this:
FTPChannelChild::DoOnStartRequest
FTPChannelChild::DoOnStopRequest -> nsUnknownDecoder::OnStopRequest -> (data is empty) -> nsUnknownDecoder::FireListenerNotifications -> nsDocumentOpenInfo::OnStartRequest -> ExternalHelperAppChild::OnStartRequest -> ExternalHelperAppChild::DivertToParent -> FTPChannelChild::DivertToParent.

However, in nsIDivertableChannel.idl the description for divertToParent() is "divertToParent is called in the child process. It can only be called during OnStartRequest()."

Enforcing this condition seems to be enough to avoid an infinite loop. The crash was fixed by bug 1436311.
Group: network-core-security
Whiteboard: [necko-triaged] → [necko-triaged][sg:dos]
Priority: -- → P2
Comment on attachment 8979355 [details] [diff] [review]
Ensure that DivertToParent is only called during OnStartRequest

Review of attachment 8979355 [details] [diff] [review]:
-----------------------------------------------------------------

thanks.
Attachment #8979355 - Flags: review?(dd.mozilla) → review+
Keywords: checkin-needed
Pushed by btara@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cb4b8d4daad
Ensure that DivertToParent is only called during OnStartRequest r=dragana
Keywords: checkin-needed
The bug was caused by the tcp connection not sending back any data, and just being closed right away.
So we get something like this:
FTPChannelChild::DoOnStartRequest
FTPChannelChild::DoOnStopRequest -> nsUnknownDecoder::OnStopRequest -> (data is empty) -> nsUnknownDecoder::FireListenerNotifications -> nsDocumentOpenInfo::OnStartRequest -> ExternalHelperAppChild::OnStartRequest -> ExternalHelperAppChild::DivertToParent -> FTPChannelChild::DivertToParent.

However, in nsIDivertableChannel.idl the description for divertToParent() is "divertToParent is called in the child process. It can only be called during OnStartRequest()."

Enforcing this condition seems to be enough to avoid an infinite loop. The crash was fixed by bug 1436311.
Attachment #8979355 - Attachment is obsolete: true
https://hg.mozilla.org/integration/mozilla-inbound/rev/8303c9587a1445920ea4c99937f995bc2d7c72df
Bug 1459666 - Ensure that DivertToParent is only called during OnStartRequest r=dragana
https://hg.mozilla.org/mozilla-central/rev/8303c9587a14
Status: UNCONFIRMED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Flags: needinfo?(valentin.gosu)
I think we should let this ride the trains given where we are in the cycle. If you feel strongly otherwise, feel free to nominate the patch for uplift with your reasoning for why.
I managed to reproduce the issue using a older version of Nightly (2018-05-07) on Ubuntu 16.04 x64. As soon as the page was loaded, the tab crashed.
I tried again using the latest Nightly and Beta 62.0b7 on the same platform and the tab didn't crash anymore. However, I didn't get any warnings that the ftp is not used at this address either. Is that expected behaviour?
If you tested with ftp://cyberghost.pro:80/pro.php then all you should see is a blank page and no crashes.
According to comment 14 and comment 15, I will mark this bug as verified.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.