Hello.
I received this report from the Debian bug system.
I'm going to apply the proposed patch to the Debian procmail package.
Not that I receive a lot of replies when I write to this address
(b...@procmail.org) but I have not completely lost my faith yet.
Thanks.
----- Forwarded message from Tero Marttila <te...@fixme.fi> -----
Date: Wed, 03 Dec 2014 21:34:46 +0200
From: Tero Marttila <te...@fixme.fi>,
To: sub...@bugs.debian.org
Subject: Bug#771958: procmail: heap overflow in getlline()
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101
Thunderbird/31.2.0
Package: procmail
Version: 3.22-20+deb7u1
Severity: important
Tags: security
We ran into a procmail segfault when dealing with a ridiculous (but legit)
procmailrc, and I tracked it down to an off-by-one error in the overflow
handling of getlline() in the presence of an overly long and suitably aligned
set of line continuations.
Please find a (very crudely tested) patch below that appears to handle this
specific example. I have only tested this patch against this specific test
case, and NOT against normal use of procmail.
The latter change (target>=end-2) should fix the root cause of this bug. The
first change (q>=end) is just to be extra sure, in case any other bug exposes
getbl() to similarly confusing parameters.
> --- procmail-3.22.orig/src/cstdio.c
> +++ procmail-3.22/src/cstdio.c
> @@ -144,7 +144,7 @@ int getbl(p,end)char*p,*end;
> /* my
> { case '\n':case EOF:*q='\0';
> return overflow?-1:p!=q; /* did we read anything at all?
> */
> }
> - if(q==end) /* check here so that a trailing backslash won't
> be lost */
> + if(q>=end) /* check here so that a trailing backslash won't
> be lost */
> q=p,overflow=1;
> *q++=i;
> }
> @@ -199,7 +199,7 @@ int getlline(target,end)char*target,*end
> if(*(target=strchr(target,'\0')-1)=='\\')
> { if(chp2!=target) /* non-empty line?
> */
> target++; /* then preserve the backslash
> */
> - if(target>end-2) /* space enough for getbl?
> */
> + if(target>=end-2) /* space enough for getbl?
> */
> target=end-linebuf,overflow=1; /* toss what we have
> */
> continue;
> }
Detailed analysis of the crash follows below. Thanks to Antti Jaakkola for
providing the input file used to debug this, and Henri Salo for advice on
reporting it.
-- Tero Marttila
> Hardware watchpoint 5: buf2[linebuf+1]
>
> Old value = 0 '\000'
> New value = 65 'A'
> 0x0000000000404abc in getbl (p=p@entry=0x88f64f "AAA", end=0x88f64f "AAA",
> end@entry=0x88f650 "AA") at cstdio.c:149
> 149 *q++=i;
> (gdb) bt
> #0 0x0000000000404abc in getbl (p=p@entry=0x88f64f "AAA", end=0x88f64f
> "AAA", end@entry=0x88f650 "AA") at cstdio.c:149
> #1 0x0000000000404c2f in getlline (target=0x88f64f "AAA", end=0x88f650 "AA")
> at cstdio.c:196
> #2 0x0000000000409dc3 in conditions (flags=flags@entry=0x615528 "",
> prevcond=prevcond@entry=0, lastsucc=lastsucc@entry=0,
> lastcond=lastcond@entry=0, skipping=skipping@entry=0, nrcond=-1,
> nrcond@entry=0) at misc.c:439
> #3 0x00000000004026a9 in mainloop () at procmail.c:678
> #4 0x0000000000403eb1 in main (argc=<optimized out>, argv=<optimized out>)
> at procmail.c:461
> (gdb) p p
> $155 = 0x88f64f "AAA"
> (gdb) p end
> $156 = 0x88f64f "AAA"
> (gdb) p q
> $157 = 0x88f651 "A"
IOW, getlline() is overflowing the buffer it is being passed:
> (gdb) p buf2
> $153 = 0x88ee50 "\nFrom:", 'a' <repeats 84 times>, "|\\\nFrom:", 'a' <repeats
> 84 times>, "|\\\nFrom:aaaaaaaaaa"...
> (gdb) p buf2 + linebuf
> $154 = 0x88f650 "AA"
I assume this is due to an off-by-one bug in getbl():
> (gdb) l 140,151
> 140 int getbl(p,end)char*p,*end; /*
> my gets */
> 141 { int i,overflow=0;char*q;
> 142 for(q=p,end--;;)
> 143 { switch(i=getb())
> 144 { case '\n':case EOF:*q='\0';
> 145 return overflow?-1:p!=q; /* did we read anything
> at all? */
> 146 }
> 147 if(q==end) /* check here so that a trailing backslash won't
> be lost */
> 148 q=p,overflow=1;
> 149 *q++=i;
> 150 }
> 151 }
When passed p = 0x88f64f, end = 0x88f650 (i.e. the last char in the buffer),
after the first read, we have q == end and the if evaluates to true, overflow
is set, but q = p is a no-op, and the unconditional *q++ then ensures that we
escape into q > end territory, and getbl() just continues copying input bytes
into the heap.
In the build I was testing with, then shows up as a SIGSEGV when the global
myenv linked list gets corrupted:
> (gdb) c
> Continuing.
> procmail: Exceeded LINEBUF
>
> Program received signal SIGSEGV, Segmentation fault.
> __strncmp_sse42 () at ../sysdeps/x86_64/multiarch/strcmp.S:260
> 260 ../sysdeps/x86_64/multiarch/strcmp.S: No such file or directory.
> (gdb) up
> #1 0x000000000040dd62 in sputenv (a=a@entry=0x412070
> "PROCMAIL_OVERFLOW=yes") at variables.c:62
> 62 if(!strncmp(a,curr->ename,eq)&&((char*)curr->ename)[eq]=='=')
> (gdb) bt
> #0 __strncmp_sse42 () at ../sysdeps/x86_64/multiarch/strcmp.S:260
> #1 0x000000000040dd62 in sputenv (a=a@entry=0x412070
> "PROCMAIL_OVERFLOW=yes") at variables.c:62
> #2 0x000000000040e0ea in setoverflow () at variables.c:148
> #3 0x0000000000404c46 in getlline (target=<optimized out>, end=0x88f650 'A'
> <repeats 87 times>) at cstdio.c:208
> #4 0x0000000000409dc3 in conditions (flags=flags@entry=0x615528 "",
> prevcond=prevcond@entry=0, lastsucc=lastsucc@entry=0,
> lastcond=lastcond@entry=0, skipping=skipping@entry=0, nrcond=-1,
> nrcond@entry=0) at misc.c:439
> #5 0x00000000004026a9 in mainloop () at procmail.c:678
> #6 0x0000000000403eb1 in main (argc=<optimized out>, argv=<optimized out>)
> at procmail.c:461
> (gdb) p curr
> $160 = (struct dynstring *) 0x4141414141414141
This is due to the `struct dynstring *` (part of myenv) situated after buf2
getting corrupted:
> (gdb) p *(struct dynstring*) 0x88f650
> $163 = {enext = 0x4141414141414141,
> ename = 'A' <repeats 79 times>, '\000' <repeats 17 times>, "<censored>",
> '\000' <repeats 37 times>, "n\000\000\000\000\000"}
The input file used to reproduce this issue is below. The trailing part of
arbitrary AAAAA's is where the heap overflow begins. This is tied to the
LINEBUF used (default 2048), and could thus probably be shorter; the final
"\\\n" just needs to suitably fall on the second-to-last position (?) in the
buf2. I assume that the code which is supposed to guard against this also has
an off-by-one bug:
> (gdb) l 199,205
> 199 if(*(target=strchr(target,'\0')-1)=='\\')
> 200 { if(chp2!=target) /*
> non-empty line? */
> 201 target++; /* then preserve the
> backslash */
> 202 if(target>end-2) /* space enough for
> getbl? */
> 203 target=end-linebuf,overflow=1; /* toss what
> we have */
> 204 continue;
> 205 }
--- cut here ---
DEFAULT=mailbox
:0:
*
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
From:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa|\
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
----- End forwarded message -----
--
To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
