qmail remote root patch

[Anton Alin-Adrian]

Anton Alin-Adrian wrote:

> Anton Alin-Adrian wrote:
>
>> Regarding latest qmail vulnerability, I coded this quickly patch. 
>> Please double-check me if I am wrong here. Forward this to 
>> freebsd-security please.
>>
>>
>> Regards,
>> Alin.
>>
>> ------------------------------------------------------------------------
>>
>> 320c320
>> <       ++pos;
>> ---
>>  
>>
>>>      if (pos>9) ++pos;
>>>   
>>> ------------------------------------------------------------------------ 
>>>
>>>
>>> _______________________________________________
>>> freebsd-hackers at freebsd.org mailing list
>>> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
>>> To unsubscribe, send any mail to 
>>> "freebsd-hackers-unsubscribe at freebsd.org"
>>>   
>>
> I forgot to mention about vuln:
>
> http://www.guninski.com/qmailcrash.html
>

Actually that was utterly wrong. I think this works:
bash-2.05b$ diff -a qmail-smtpd.c qmail-smtpd-patched.c
318a319
 >       ++pos;
320d320
<       ++pos;


The patched function will look like:
void blast(hops)
int *hops;
{
  char ch;
  int state;
  int flaginheader;
  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);
  }
}


So what I did is move ++pos; into the if (pos < 9) block. Originally it 
is right after the } ending that block.

This works if pos gets incremented as 
pos=1,2,.....9,10,...,max,...,upper-overflow(negative).

This utterly fails if pos is not incremented like that.

Any ideas? I think it works, after a first look at the incrementation loop.

Sorry for all other mails, I am stressed . (need to calm down i know)

Alin.