Thursday, September 23, 2010

Write safe code

In these days, I come cross a few vulnerability of squid. Here are some lessons learned:


(1) strcmp

If you pass the strcmp with NULL pointer, the behavior is undefined and program may crash. Also check the NULL, '\0' and string can be trivial. This assumes that the str1 and str2 is end with '\0':


int compare(char* str1, char* str2)

{

         if (str1==NULL || str2==NULL)

         {

                if (str1==str2)

                     return 0;

                 if (str1==NULL)

                     return -1;

                 if (str2==NULL)

                     return 1;

          }

           return strcmp(str1,str2);

}


(2) check the minor major version in HTTP header.
Squid was using this code to get numeric version number from HTTP header (HTTP/1.1...):


//assume the data stored in buffer, assume we only care about major digit now..

int maj=-1;



if (buffer see line end)

  maj=1;

else

   return;


for (pos=verStart;  isdigit(buffer[pos]); pos++)

{

          maj = maj * 10;

          maj = maj + (hmsg->buf[i]) - '0';

}


//The maj should never be -1 until it is overflow at 65536

assert(maj!=-1)

(3) Recently, there is a vulnerablity in bzip2 code
int N, result;
while (buffer not end)
{
       //read buffer;
       result+=N*2;
}

Here the result is signed integer and it may overflow, which cause undefined behavior.

No comments: