Closed Bug 721852 Opened 12 years ago Closed 12 years ago

Only accept 221 and 500 after QUIT

Categories

(MailNews Core :: Networking: SMTP, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox-esr10 unaffected, thunderbird14 fixed, thunderbird15 fixed, thunderbird-esr1014+ fixed)

RESOLVED FIXED
Thunderbird 16.0
Tracking Status
firefox-esr10 --- unaffected
thunderbird14 --- fixed
thunderbird15 --- fixed
thunderbird-esr10 14+ fixed

People

(Reporter: pmbmo1, Assigned: Bienvenu)

Details

(Keywords: sec-moderate)

Attachments

(3 files)

User Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US) AppleWebKit/534.3 (KHTML, like Gecko) Chrome/6.0.472.63 Safari/534.3

Steps to reproduce:

Replied to Thunderbird with non rfc compliant reply codes during SMTP. rfc821 states that a server can reply to a QUIT only with 221 or 500.


Actual results:

If an SMTP server replies with a 220 code to the QUIT smtp command sent by Thunderbird, Thunderbird will restart and execute the transaction again starting with a new EHLO line. However, at that point the buffers containing the mail info have been cleared already. Thus, Thunderbird will send random? memory contents to the server.


Expected results:

Thunderbird should only accept 221 or 500 codes as replies to the QUIT command as stated in the rfc.
Component: Security → Networking: SMTP
Product: Thunderbird → MailNews Core
QA Contact: thunderbird → networking.smtp
seems like somewhere between an sg:low and an sg:moderate to me, leaning towards sg:low since it sounds like the attacker can't control which memory is disclosed to the server
Group: core-security
Some more details I could find out after going over the latest source:

As explained the problem arises if the server sends a 220 after the client sent the QUIT.

SendQuit sets m_nextState to SMTP_RESPONSE and m_nestStateAfterResponse to SMTP_DONE

thus, ProcssProtocolState will dipatch SmtpResponse first and read the 220 in turn resulting in a restart of the SMTP statemachine from the beginning. 

After EHLO and MAIL FROM thunderbird will send the list of RCPT TOs. Hoever, this list has been exhaused previously and the pointer (m_addreses) is still pointing to the end of the list. m_addresssesLeft is 0 at that point. Then Thunderbird will continue sending RCPT TOs as long as --m_addressesLeft > 0, m_addresssesLeft is a PRUint32 thus -- is a pretty large postitive number and thus > 0.
Thunderbird will thus go on and send chunks of memory after m_addresses until it his a NULL byte and then send the next chunk. All as arguments to the RCPT TO command.

I also attached python poc (just hacked together) but should demonstrate the problem.

Hope that helps.
Attached file Output from POC server
I'm not sure you're POC is working well. Within TB 10.0.1, I just see a failure to send when using your server. I've attached the console output from your POC server and I note that there is an exception afterwards. 

Could be my python setup on OS X.
Hi, 
regarding your attachment. That data that the server prints out is actually the memory after the m_addresses array on the heap. I think this should not happen. Sometimes, Thunderbird decides to close the connection (which is what you are seeing). In other instances, Thunderbird segfaults b/c it tries to access unmapped data.

Running the poc multiple times should yield a segfault case eventually (at least that's the behavior i can observe here for tb 9.0.1 on windows).

cheers
--m
Marking this as confirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
MBanner, can someone from the TB team look at this?
JB: please find an appropriate assignee on the Thunderbird team for this security bug.
Assignee: nobody → jb
Keywords: sec-moderate
(In reply to Daniel Veditz [:dveditz] from comment #8)
> JB: please find an appropriate assignee on the Thunderbird team for this
> security bug.

Sure. Will do, but you could have cc'ed me so I'm aware of the request ;-)
Assignee: jb → dbienvenu
Attached patch possible fixSplinter Review
Bugzilla is not letting me look at the fake server attachment, but if I'm understanding the comments correctly, I think this should fix it.
Attachment #634637 - Flags: review?(mbanner)
Comment on attachment 634637 [details] [diff] [review]
possible fix

Looks good, thanks David.

I think this is safe enough to land on all branches - include in the next beta and then in the builds that we'll start next week?
Attachment #634637 - Flags: review?(mbanner) → review+
fixed on trunk - http://hg.mozilla.org/comm-central/rev/c4e3fb7696f8
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 16.0
(In reply to Mark Banner (:standard8) from comment #11)

> 
> I think this is safe enough to land on all branches - include in the next
> beta and then in the builds that we'll start next week?

yeah, I think so to.
Comment on attachment 634637 [details] [diff] [review]
possible fix

[Triage Comment]
Ok, a=me for branches, I'm assuming we want this for esr as well.
Attachment #634637 - Flags: approval-comm-esr10+
Attachment #634637 - Flags: approval-comm-beta+
Attachment #634637 - Flags: approval-comm-aurora+
http://hg.mozilla.org/releases/comm-aurora/rev/1dab1914bb6b
http://hg.mozilla.org/releases/comm-beta/rev/cafe876e6e3a

I don't have a comm-esr tree on my mac, so it would be helpful if you could land it there, Mark.
(In reply to David :Bienvenu from comment #15)
> http://hg.mozilla.org/releases/comm-aurora/rev/1dab1914bb6b
> http://hg.mozilla.org/releases/comm-beta/rev/cafe876e6e3a

That comm-beta changeset was for a different bug and this didn't actually land on beta, so I've now landed it on beta:

https://hg.mozilla.org/releases/comm-beta/rev/14f55fb00fff

I've also landed it on esr:

https://hg.mozilla.org/releases/comm-esr10/rev/4885f43b672c
Dan should this one be open now ?
Group: core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: