In ssmtp.c the function fd_gets more or less looks like this: char *fd_gets(char *buf, int size, int fd) { while((i < size) && (fd_getc(fd, &c) == 1)) { buf[i++] = c; } buf[i] = (char)NULL; return(buf); } Coming out of the loop, i can be size, causing a 0-byte to be written past the end of the buffer. There are also lots of "char c = (char)NULL;" and "char *p = (char)NULL;" occurrences that may be indicative of careless programming and may warrant a code review.
I agree on the off-by-one error, but initializing variables to NULL before using them seems a rather good practice to me, as it allows to find some bugs more easily. We'll probably have to contact upstream.
Created attachment 162630 [details, diff] Proposed misc fixes In addition to fixing the "(char)NULL" things and the off-by-one, this also addresses a problem in from_format() that caused a call to strdup on a local buffer with uninitialized contents. This last problem was introduced in 2.62 and was the reason I started looking at the source code in the first place (the From: line wasn't properly formatted). I have not looked at the rest of the source, so I'm not claiming this is a complete solution.
Concerning the potential off-by-one: There is no off-by-one error if 'int size' is not the buffer size, but the maximum number of characters the buffer can contain (not counting the NUL). From my reading of the code, this is what happens: The only call of that function is here: int smtp_read(int fd, char *response) { do { if(fd_gets(response, BUF_SZ, fd) == NULL) { ... smtp_read() is called at several places, but all buffers that are passed as 'response' have been allocated as 'BUF_SZ + 1'. Am I missing something?
You're right about the buffer size. Now if the strdup is not something that can be abused, then this can be demoted to a regular ssmtp bug.
Created attachment 163507 [details, diff] Updated patch. Removed fix for off-by-one that wasn't an off-by-one. Updated patch.
Created attachment 165005 [details, diff] ssmtp-unitialized-strdup.patch Just the security-relevant hunk.
Opening to the public, please commit with the patch.
CVE-2008-3962 has been assigned.
2.62-r3 is inCVS.
As Tomas Hoger pointed out, this has been a re-introduction of bug 127592 since that patch was dropped in the ebuild when 2.62 was bumped. 2.61 is also affected, but we patched it in 2006.
(In reply to comment #10) > As Tomas Hoger pointed out, this has been a re-introduction of bug 127592 since > that patch was dropped in the ebuild when 2.62 was bumped. > > 2.61 is also affected, but we patched it in 2006. > only 2.61-r2 and 2.61-r31 where patched, but not -2.61-r30 which i apparantly used as a base for 2.62, thus the patch got dropped for 2.62. 2.61-r30 was always p.masked iirc.
Arches, please test and mark stable: =mail-mta/ssmtp-2.62-r3 Target keywords : "alpha amd64 arm hppa ia64 m68k ppc ppc64 s390 sh sparc x86"
Stable for HPPA.
amd64/x86 stable
alpha/ia64/sparc stable
ppc and ppc64 stable
It was only stable for a short timeframe and the issue is almost impossible to exploit. My vote is NO glsa.
no too, and closing.