Closed Bug 1525267 (CVE-2019-9806) Opened 2 years ago Closed 1 year ago

FTP triggers undismissable auth prompts

Categories

(Firefox :: Security, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 67
Tracking Status
firefox-esr60 --- wontfix
firefox65 --- wontfix
firefox66 --- verified
firefox67 --- verified

People

(Reporter: Anca, Assigned: johannh)

References

Details

(Keywords: csectype-dos, sec-low, Whiteboard: [post-critsmash-triage][adv-main66+])

Attachments

(2 files)

Attached image 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.
Flags: needinfo?(jhofmann)
Group: core-security-release

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?

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

Assignee: nobody → jhofmann
Status: NEW → ASSIGNED
Component: Networking: FTP → Security
Flags: needinfo?(jhofmann)
Priority: -- → P1
Product: Core → Firefox
Group: core-security-release → firefox-core-security

ftp urls have hostnames and origins.

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

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.

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

(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.

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.

Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

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?

Flags: needinfo?(dveditz)

(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...

(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.

Flags: needinfo?(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).

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

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

Flags: needinfo?(jhofmann) → needinfo?(ryanvm)

Yes, one request per bug please.

Flags: needinfo?(ryanvm)

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
Attachment #9047668 - Flags: approval-mozilla-beta?

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.

Attachment #9047668 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: qe-verify+
Whiteboard: [post-critsmash-triage]
QA Whiteboard: [qa-triaged]

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).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main66+]
Alias: CVE-2019-9806
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.