Share via

Buffer overruns - keeping the inside in

  • Article

Ah, another “Patch Tuesday” or “Update Tuesday” as we are supposed to call it. Patches have traditionally been replacements for only part of files and we typically replace multiple files.

So, last blog, I wittered on about why buffer overflows were a bad thing. Of course, you already knew that they were a bad thing but you might not have been that familiar with legacy exploit techniques. The new techniques are a bit different but they still require overwrites of the stack or heap and avoidance is the same either way.

So, let us consider the example that we had before:

MyFunc(int iStream, char* buffer)

{

   char szLocalBuffer[100];

  

   strcpy(szLocalBuffer, buffer);

   …

}

This code is clearly an accident waiting to happen since we are doing an unbounded copy. One obvious improvement would be to use strncpy which only copies a certain number of characters. Here is one version – it is polymorphic but this is the simplest.

char *strncpy( char *strDest, const char *strSource, size_t count );

size_t is an unsigned integer which doesn’t really matter to us since we are going to have a constant in there.

Now, the more eagle eyed among you may have spotted that MSDN says that you should use strncpy_s instead of strncpy but there is a lot of legacy code and legacy coders out there so let us pass that by for a moment.

So, the code becomes:

strncpy(szLocalBuffer, buffer, 100);

Much better. Safe? No. What if I pass in a string with no null at the end or a null after the 100th position? Well, then Buffer has no null at the end. That could cause later code to walk off the end of the buffer. It might AV. It might alter memory including the return address. Best to make sure that there is a null on the end. I would make sure that there was one – and there are a number of ways of doing so. However, this highlights a potential problem. I said that the string should not be more than 100 characters. I didn’t say if that included the null or if the string could contain nulls. Sloppy specification can lead to all manner of errors including security holes.

Let us imagine that we have specified that the string must be no longer than 100 ANSI characters and null terminated taking the total length to 101 – before we didn’t specify if we were talking about 1 or 2 byte characters and that matters rather a lot.

So, next try.

  MyFunc(int iStream, char* buffer)

  {

     char szLocalBuffer[101] = { 0 }; // Set the buf to all nulls

    strncpy(szLocalBuffer, buffer, 100);

    …

  }

The buffer may contain nonsense but it is nonsense in the buffer and not sprawled over the stack. However, the 100 character limit is a bit arbitrary. Typically you would want a variable length string but to be sensible, we should limit the buffer a bit. If we allow a buffer of 1 GB then it won’t be hard to take down the process. 32K (including the null) is plenty. Assume that we are talking 32 bit here. So, Take 3.

  MyFunc(int iStream, char* buffer, int iBufLen)

  {

  

  if (iBuflen> 32767)

  {

     // handle error somehow

    …

  }

  // let us assume that we have allocated 32K byte on the heap.

  // It won’t fragment memory badly and we won’t keep it for that long

  szLocalBuffer[iBuflen]=0; // null at the end whatever

  strncpy(szLocalBuffer, buffer, iBuflen);

  …

}

That looks pretty safe, doesn’t it? Exercise for the reader. What happens when I tell the routine that my string is 0xFFFFFFFF bytes long?

 

Answers next post unless someone beats me to it.

Comments

  • Anonymous
    June 18, 2007
    "What happens when I tell the routine that my string is 0xFFFFFFFF bytes long?" As you declared the length argument as an int then you'll get an integer overflow.  The length will wrap around to become -1.

  • Anonymous
    June 18, 2007
    Correct. And the effect of this will be that it will try to copy FFFFFFFF bytes - doomed to failure as that is more than the process space. However, if there is a null in string, then it will stop there. Give it the right string and you have just overwritten the stack again without necessarily crashing the process. You now can exploit the function. So, mixing signed and unsigned in this context is dangerous. There is a reason that the compiler has a warning for it. Next post, I will be talking about this and stack cookies and some other things on the same lines. Thanks, Will Mark