Sendmail Silently-Patched Memory Leak [Deprecated]

Update:
Regarding my blog on the memory leak in Sendmail, I was wrong.
The patch fixes a minor resource-depletion issue and does not appear to have any security consequences.
I apologize for the mistake, and would like to thank Eric Allman from the sendmail team for the correction.

Ido Kanner,
SecuriTeam

Sendmail silently fixed a memory leak in the recent multiple vulnerabilities patch.

The problem occurs when a buffer is set to NULL instead of freeing its memory, causing the data to be marked as being used even though there is no variable that stores the data address.

This happens when the original (buf0) buffer and the buf buffer have different addresses.

The fix was as following:
In the file: contrib/sendmail/src/conf.c


- if (buf == NULL)
- {
- buf = buf0;
- bufsize = sizeof buf0;
- }
+ buf = buf0;
+ bufsize = sizeof buf0;

for (;;)
{
@@ -5281,8 +5278,8 @@
(void) sm_io_fprintf(smioerr, SM_TIME_DEFAULT,
"%s: %s\n", id, newstring);
#endif /* LOG */
- if (buf == buf0)
- buf = NULL;
+ if (buf != buf0)
+ sm_free(buf);
errno = save_errno;
return;
}

This advisory can be found here: http://www.securiteam.com/unixfocus/5SP0M0UI0G.html

Share
  • Pingback: DivisionByZero WebLog»Blog Archive » kwetsbaarheid in sendmail (update 2)

  • Eric Allman

    This advisory is incorrect. There was no memory leak. We did change that code, but for a totally unrelated reason.

    If you look at the code just a little more closely, you’ll see that in 8.13.5 buf0 is a local variable. The variable “buf” could point either to this buffer or to allocated memory. Freeing the local variable would trash memory.

    I believe we are owed a (public) apology. It is irresponsible to publish an advisory like this without even verifying the problem.

  • http://www.security-protocols.com Tom Ferris

    Yeah, false information is being spread on this blog. Not cool…

  • sunshine

    Whether Ido is right or wrong, he gave the code for your viewing. Didn’t he?

  • http://BeyondSecurity.com ido

    Lets see,
    We have buf, that will get a new memory address allocated (view the advisory we published, the link exists in my blog), now until the rewrite, that memory never got freed, or returned by the function.
    Now, if you ask how the memory leak could happen, here is the explanation:
    "if (n < bufsize)
    break;"

    That line will exit the for loop without freeing the allocated memory, while if that line will not happen, it will free the memory and allocate a new one...
    Unless I'm missing something, I think I just proved that there is a memory leak.
    Now if you “fixed” it without see that problem I think we have a problem on our hands (and thats if there is a memory leak...)

  • Philip Guenther

    The previous code didn’t leak memory because the ‘buf’ and ‘bufsize’ variables were static, so if a buffer was allocated in sm_syslog(), ‘buf’ would be left non-NULL and pointing to that allocated buffer and the next invocation of sm_syslog() would see that same pointer when it started.

  • Pingback: SecuriTeam Blogs » Code Red: Opera Cannot Handle Insufficent Disk Space and the SecuriTeam vs. Sendmail armed conflict