Date: Tue, 20 Jan 2004 17:07:18 +0200 From: Anton Alin-Adrian <aanton@reversedhell.net> To: freebsd-hackers@freebsd.org, freebsd-security@freebsd.org Subject: short analysys of qmail integer overflow bug - let there be light Message-ID: <400D4426.1040302@reversedhell.net>
next in thread | raw e-mail | index | archive | help
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
+* <CRLF> 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) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?400D4426.1040302>