1525267 - (CVE-2019-9806) FTP triggers undismissable auth prompts

Status: VERIFIED FIXED

(Firefox :: Security, defect, P1)

Description

[Anca Soncutean, Desktop QA]

Attachment: screencast issue.gif

[Affected versions]:

• 67.0a1 (2019-02-04)
• 66.0b5 (20190204181317)
• 65.0 (20190124174741)

[Affected platforms]:

• Windows 10 x64

[Steps to reproduce]:

1. Setup a simple FTP server
2. Use the following test case
3. Access the above test case via FTP server

[Expected result]:

• The auth prompt should not be displayed once more.

[Actual result]:

• Auth prompt is successively displayed, without any means to be dismissed.

[Regression range]:

• Not a regression, I can see this issue way back to Fx 31.0a1.

[Additional Notes]:

• Firefox can’t be escaped, only by ending its process.
• This behavior is not reproducible on Chrome.
• In order to create the FTP server, I used FileZilla.

Comment 1

[:Gijs (he/him)]

FWIW, I don't understand, off-hand, why the auth prompts from FTP aren't being subjected to the same limits we added for http auth prompts. I'm assuming it's the same dialog. Hopefully Johann knows?

Comment 2

[Johann Hofmann [:johannh]]

Pretty sure this is because we base it on the hostname. I can reproduce and will fix it later.

Comment 3

[Daniel Veditz [:dveditz]]

ftp urls have hostnames and origins.

Comment 4

[Johann Hofmann [:johannh]]

Yeah, I think it's more because of the localhost part. We might still want to fix it.

Comment 5

[Johann Hofmann [:johannh]]

Ok, Dan is right this is not related to hostname issues, but because we seem to take a different code path for prompting with ftp: https://searchfox.org/mozilla-central/source/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#758

Gah.

Comment 6

[Matthew N. [:MattN]]

See also bug 565582 comment 0. The auth prompt story is messy.

Comment 7

[:Gijs (he/him)]

(In reply to Johann Hofmann [:johannh] from comment #5)

Ok, Dan is right this is not related to hostname issues, but because we seem to take a different code path for prompting with ftp: https://searchfox.org/mozilla-central/source/netwerk/protocol/ftp/nsFtpConnectionThread.cpp#758

Gah.

Could we move FTP over to use the same stuff as HTTP? Presumably the mChannel reference here could be used to find the loadgroup and document / tabchild etc.

Comment 8

[Mark Straver]

Maybe a dumb question, but why is html being parsed when retrieved over ftp to begin with?

Seems to me the root issue here is that the html is trying to use the ftp protocol as-if it was http, which it isn't, causing a flurry of ftp connections that the server eventually refuses (rate limiting, concurrent connection limits, etc.)

If this parsing isn't necessary, then this whole problem could be avoided by simply either displaying the source or (probably better) simply offering a download of the file. It's FTP after all.

Comment 9

[:Gijs (he/him)]

(In reply to Mark Straver from comment #8)

Maybe a dumb question, but why is html being parsed when retrieved over ftp to begin with?

I expect you could reproduce this issue if you hosted the html page on http:, but sourced images over ftp:.

Seems to me the root issue here is that the html is trying to use the ftp protocol as-if it was http, which it isn't

Well, we display HTML content for file: protocol use as well (and data:, and blob:, and chrome:, and resource:, and about:, and moz-extension:, and ...). I don't know why you seem to believe it's only reasonable to do so over the http: protocol.

While there is some movement towards removing support for browsing HTML documents served over FTP in all browsers, we're not in a place where we can just pull such support unilaterally.

Comment 10

[Mark Straver]

Well, we display HTML content for file: protocol use as well (and data:, and blob:, and chrome:,
and resource:, and about:, and moz-extension:, and ...). I don't know why you seem to believe it's
only reasonable to do so over the http: protocol.

I don't believe it's only reasonable to use http: -- I never said that. But I do believe it's unreasonable to treat a remote transfer protocol like ftp: the same way as http: when it's clearly not suited or designed for the protocol behavior typical to http: (which the other mentioned protocols are).

I expect you could reproduce this issue if you hosted the html page on http:, but sourced images
over ftp:.

You're absolutely right. So the issue is linked resources, not the html itself.

Comment 11

[Johann Hofmann [:johannh]]

Attachment: Bug 1525267 - Move auth prompt rate limiting to nsIAuthPrompt2.promptAuth. r=MattN

This also changes the name of 'canceledAuthenticationPromptCounter' to account for the
fact that we no longer count up when the prompt was cancelled, but when it was shown.

Comment 12

[Johann Hofmann [:johannh]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=70e26d55c37ba4db365a5b42c5193179aeddc9aa

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/integration/autoland/rev/6933acee1ad9aba811d05eb9d820d9397f45bd2a
https://hg.mozilla.org/mozilla-central/rev/6933acee1ad9

Comment 14

[Johann Hofmann [:johannh]]

Considering that this is really just an extension of bug 377496 and they were both fixed in the same release, we can probably unhide this. Do you agree, Dan?

Comment 15

[:Gijs (he/him)]

(In reply to Johann Hofmann [:johannh] from comment #14)

Considering that this is really just an extension of bug 377496 and they were both fixed in the same release, we can probably unhide this. Do you agree, Dan?

bug 1523249 is still hidden though, and this fix is related. It's probably too late to upload all the auth fixes to 66, but I'd be more comfortable holding off on opening this up for a bit...

Comment 16

[Johann Hofmann [:johannh]]

(In reply to :Gijs (he/him) from comment #15)

(In reply to Johann Hofmann [:johannh] from comment #14)

Considering that this is really just an extension of bug 377496 and they were both fixed in the same release, we can probably unhide this. Do you agree, Dan?

bug 1523249 is still hidden though, and this fix is related. It's probably too late to upload all the auth fixes to 66, but I'd be more comfortable holding off on opening this up for a bit...

Ok, fair enough, I think you're right. Let's keep it hidden for now.

Comment 17

[Daniel Veditz [:dveditz]]

(In reply to :Gijs (he/him) from comment #9)

I expect you could reproduce this issue if you hosted the html page on http:, but sourced images over ftp:.

No, we disabled that in Firefox 61 (bug 1404744).

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Should this (and bug 377496) be nominated for Beta uplift?

Comment 19

[Johann Hofmann [:johannh]]

Asked for uplift in the other bug, do you need another request in this bug?

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Yes, one request per bug please.

Comment 21

[Johann Hofmann [:johannh]]

Comment on attachment 9047668 [details]
Bug 1525267 - Move auth prompt rate limiting to nsIAuthPrompt2.promptAuth. r=MattN

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: None
• User impact if declined: Protections introduced in bug 377496 can be worked around by using FTP
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See comment 0, also, we should check that HTML basic auth sites are still working as expected.
• List of other uplifts needed: Bug 377496
• Risk to taking this patch: Medium
• Why is the change risky/not risky? (and alternatives if risky): This is basically just moving some code that has well baked in bug 377496 and we have tests for the http variant, but it still hasn't baked a lot on Nightly
• String changes made/needed: None

Comment 22

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9047668 [details]
Bug 1525267 - Move auth prompt rate limiting to nsIAuthPrompt2.promptAuth. r=MattN

Uplift along with the patch in bug 377496.

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/666f0ce4c40f746e8623a540f73565b050865c94

Comment 24

[Anca Soncutean, Desktop QA]

I can confirm that this issue is fixed (also checked several HTML basic auth sites) on Windows 10 x64 with 66.0b14 and 67.0a1(2019-03-07).

Comment 25

[Johann Hofmann [:johannh]]

Awesome, thanks!