Ted,

You are right. The second length check was wrongly removed. Below
follows the correct patch:

======= spamd-synchdr.patch start ==========
--- sync.c.old Tue Dec 25 22:55:07 2007
+++ sync.c Wed Dec 26 08:02:38 2007
@@ -273,7 +273,8 @@

/* Ignore invalid or truncated packets */
hdr = (struct spam_synchdr *)buf;
- if (hdr->sh_version != SPAM_SYNC_VERSION ||
+ if (len < sizeof(struct spam_synchdr) ||
+ hdr->sh_version != SPAM_SYNC_VERSION ||
hdr->sh_af != AF_INET ||
len < ntohs(hdr->sh_length))
goto trunc;

======= spamd-synchdr.patch end ==========

--
DG


On Dec 27, 2007 12:58 AM, Ted Unangst <ted.unangst@gmail.com> wrote:
> On 12/26/07, Diego Giagio <dgiagio@gmail.com> wrote:
> > This patch corrects an improper length check of spamd sync packets
> > header. AFAIK it doesn't allow remote code execution but may lead to
> > undefined behaviour in some circustancies. Also follows a simple
> > proof-of-concept program to trigger the vulnerability.
>
> if you are letting the bad people send packets to your spamd sync
> port, you're already in trouble.
> adding the first check below is probably reasonable, but the second
> len check should be kept too.
>
>
> > ======= spamd-synchdr.patch start ==========
> > --- sync.c.old Tue Dec 25 22:55:07 2007
> > +++ sync.c Tue Dec 25 22:58:39 2007
> > @@ -273,9 +273,9 @@
> >
> > /* Ignore invalid or truncated packets */
> > hdr = (struct spam_synchdr *)buf;
> > - if (hdr->sh_version != SPAM_SYNC_VERSION ||
> > - hdr->sh_af != AF_INET ||
> > - len < ntohs(hdr->sh_length))
> > + if (len < sizeof(struct spam_synchdr) ||
> > + hdr->sh_version != SPAM_SYNC_VERSION ||
> > + hdr->sh_af != AF_INET)
> > goto trunc;
> > len = ntohs(hdr->sh_length);
> >
> > ======= spamd-synchdr.patch end ==========
> >
> > --
> > DG