From owner-freebsd-hackers@FreeBSD.ORG Tue Jan 20 07:10:39 2004 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id DAFD716A4CE for ; Tue, 20 Jan 2004 07:10:38 -0800 (PST) Received: from reversedhell.net (reversedhell.net [216.240.143.80]) by mx1.FreeBSD.org (Postfix) with SMTP id C33E743D95 for ; Tue, 20 Jan 2004 07:07:08 -0800 (PST) (envelope-from aanton@reversedhell.net) Received: (qmail 17431 invoked from network); 20 Jan 2004 15:08:59 -0000 Received: from unknown (HELO reversedhell.net) (81.196.32.25) by ns1.1plan.net with SMTP; 20 Jan 2004 15:08:59 -0000 Message-ID: <400D4426.1040302@reversedhell.net> Date: Tue, 20 Jan 2004 17:07:18 +0200 From: Anton Alin-Adrian User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.6b) Gecko/20031212 Thunderbird/0.4 X-Accept-Language: en-us, en MIME-Version: 1.0 To: freebsd-hackers@freebsd.org, freebsd-security@freebsd.org Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: short analysys of qmail integer overflow bug - let there be light X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jan 2004 15:10:39 -0000 Hey folks. There are rumors out there that setting /var/qmail/control/databytes to a reasonable value (for example 16384 = 16MB) will prevent the possibility of exploitation regarding the integer overflow in function blast(). That is not true. This is how blast() is called: void smtp_data() { int hops; unsigned long qp; char *qqx; if (!seenmail) { err_wantmail(); return; } if (!rcptto.len) { err_wantrcpt(); return; } seenmail = 0; if (databytes) bytestooverflow = databytes + 1; if (qmail_open(&qqt) == -1) { err_qqt(); return; } qp = qmail_qp(&qqt); out("354 go ahead\r\n"); received(&qqt,"SMTP",local,remoteip,remotehost,remoteinfo,fakehelo); blast(&hops); hops = (hops >= MAXHOPS); if (hops) qmail_fail(&qqt); qmail_from(&qqt,mailfrom.s); qmail_put(&qqt,rcptto.s,rcptto.len); qqx = qmail_close(&qqt); if (!*qqx) { acceptmessage(qp); return; } if (hops) { out("554 too many hops, this message is looping (#5.4.6)\r\n"); return; } if (databytes) if (!bytestooverflow) { out("552 sorry, that message size exceeds my databytes limit (#5.3.4)\r\n"); return; } if (*qqx == 'D') out("554 "); else out("451 "); out(qqx + 1); out("\r\n"); } So you see, the input value is only checked against the databytes limit *after* the function blast() is called. The overflow resides in function blast(), thus even setting databytes to 1 (lowest possible value) will not prevent the overflow to happen. People should not comment a bug without reading the code. (not from this mailing list). The simplest way to fix is to define pos variable as unsigned int instead of int, in the blast() function. What happens in blast()? Here what happens: Input is read byte by byte, and if input byte != '\n', then pos is incremented. This is a neverending loop, without any logical tests. Thus, pos gets an "upper-bounds overflow" and becomes negative (because it is a signed integer). If it is defined as unsigned, it simply becomes 0, and keeps being incremented in a neverending circle loop, untill the first '\n' is met. This renders any hack attack useless. DOS is possible, but DOS is possible in many other more effective ways. It's the way of the Internet protocols. This is the original blast() function: void blast(hops) int *hops; { char ch; int state; int flaginheader; /* my comment here: unsigned int pos */ int pos; /* number of bytes since most recent \n, if fih */ int flagmaybex; /* 1 if this line might match RECEIVED, if fih */ int flagmaybey; /* 1 if this line might match \r\n, if fih */ int flagmaybez; /* 1 if this line might match DELIVERED, if fih */ state = 1; *hops = 0; flaginheader = 1; pos = 0; flagmaybex = flagmaybey = flagmaybez = 1; for (;;) { substdio_get(&ssin,&ch,1); if (flaginheader) { if (pos < 9) { if (ch != "delivered"[pos]) if (ch != "DELIVERED"[pos]) flagmaybez = 0; if (flagmaybez) if (pos == 8) ++*hops; if (pos < 8) if (ch != "received"[pos]) if (ch != "RECEIVED"[pos]) flagmaybex = 0; if (flagmaybex) if (pos == 7) ++*hops; if (pos < 2) if (ch != "\r\n"[pos]) flagmaybey = 0; if (flagmaybey) if (pos == 1) flaginheader = 0; } ++pos; if (ch == '\n') { pos = 0; flagmaybex = flagmaybey = flagmaybez = 1; } } switch(state) { case 0: if (ch == '\n') straynewline(); if (ch == '\r') { state = 4; continue; } break; case 1: /* \r\n */ if (ch == '\n') straynewline(); if (ch == '.') { state = 2; continue; } if (ch == '\r') { state = 4; continue; } state = 0; break; case 2: /* \r\n + . */ if (ch == '\n') straynewline(); if (ch == '\r') { state = 3; continue; } state = 0; break; case 3: /* \r\n + .\r */ if (ch == '\n') return; put("."); put("\r"); if (ch == '\r') { state = 4; continue; } state = 0; break; case 4: /* + \r */ if (ch == '\n') { state = 1; break; } if (ch != '\r') { put("\r"); state = 0; } } put(&ch); } } One can see that pos is later used as an index for memory location. And that's all folks. :) I say it is exploitable. Just an opinion. Cheers to all, Alin-Adrian Anton. Below there is a small rfc821 line too long implementing patch: --- qmail-smtpd.c.orig Mon Jun 15 13:53:16 1998 +++ qmail-smtpd.c Mon Jan 19 23:29:35 2004 @@ -1,3 +1,15 @@ +/* +* This is a patched version of qmail, implementing RFC 821 regarding text line limitations. +* Developed by Alin-Adrian Anton (aanton@reversedhell.net,burebista@lasting.ro) +* +* You may remove this banner if it annoys you. This patch is public domain, for the +* benefit of the community. +* +* It also fixes an integer overflow in the blast() function. + NOTE: it implements the most relaxed RFC821, as it is specified there. +*/ + + #include "sig.h" #include "readwrite.h" #include "stralloc.h" @@ -48,7 +60,6 @@ void die_control() { out("421 unable to read controls (#4.3.0)\r\n"); flush(); _exit(1); } void die_ipme() { out("421 unable to figure out my IP addresses (#4.3.0)\r\n"); flush(); _exit(1); } void straynewline() { out("451 See http://pobox.com/~djb/docs/smtplf.html.\r\n"); flush(); _exit(1); } - void err_bmf() { out("553 sorry, your envelope sender is in my badmailfrom list (#5.7.1)\r\n"); } void err_nogateway() { out("553 sorry, that domain isn't in my list of allowed rcpthosts (#5.7.1)\r\n"); } void err_unimpl() { out("502 unimplemented (#5.5.1)\r\n"); } @@ -58,7 +69,7 @@ void err_noop() { out("250 ok\r\n"); } void err_vrfy() { out("252 send some mail, i'll try my best\r\n"); } void err_qqt() { out("451 qqt failure (#4.3.0)\r\n"); } - +void err_longline() { out("500 Line too long, please read RFC 821.\r\n"); flush(); _exit(1); } stralloc greeting = {0}; @@ -293,10 +304,46 @@ void blast(hops) int *hops; { + +/* +*RFC 821 August 1982 +* Simple Mail Transfer Protocol +* +* text line +* +* The maximum total length of a text line including the +* is 1000 characters (but not counting the leading +* dot duplicated for transparency). +* +* recipients buffer +* +* The maximum total number of recipients that must be +* buffered is 100 recipients. +* +* +* **************************************************** +* * * +* * TO THE MAXIMUM EXTENT POSSIBLE, IMPLEMENTATION * +* * TECHNIQUES WHICH IMPOSE NO LIMITS ON THE LENGTH * +* * OF THESE OBJECTS SHOULD BE USED. * +* * * +* **************************************************** +* +* Errors due to exceeding these limits may be reported by using +* the reply codes, for example: +* +* 500 Line too long. +* +* 501 Path too long +* +* 552 Too many recipients. +* +* 552 Too much mail data. +*/ char ch; int state; int flaginheader; - int pos; /* number of bytes since most recent \n, if fih */ + unsigned int pos; /* number of bytes since most recent \n, if fih */ int flagmaybex; /* 1 if this line might match RECEIVED, if fih */ int flagmaybey; /* 1 if this line might match \r\n, if fih */ int flagmaybez; /* 1 if this line might match DELIVERED, if fih */ @@ -317,7 +364,8 @@ if (pos < 2) if (ch != "\r\n"[pos]) flagmaybey = 0; if (flagmaybey) if (pos == 1) flaginheader = 0; } - ++pos; + if (++pos>65535-1) err_longline(); /* will bail out nicely with err 500 */ + if (ch == '\n') { pos = 0; flagmaybex = flagmaybey = flagmaybez = 1; } } switch(state) {