Partager via

Something else to look out for when reviewing code

  • Article

From: The Learning from Mistakes Dept.

 

A few months back eEye found an exploitable buffer overrun in Symantec’s Remote Management software what caught my eye was the nature of the bug, and I think this is coding construct we should all learn from.

 

You’re no doubt familiar with issues with strncpy and strncat, I’ve blogged about it in the past but this bug is a new twist, it’s an integer underflow when calculating the buffer size:

 

strncat(dst, src, N – strlen(src));

 

N is a constant, probably the len of the destination buffer, but the real issue is the attacker controls src, so he can make it any length he wants, and if src is longer than N, then, well you know the rest!

 

So when you’re reviewing code, look out for this little gem.

Comments

  • Anonymous
    October 30, 2006
    using /SAFESEH might have saved them too. http://metasploit.com/svn/framework3/trunk/modules/exploits/windows/antivirus/symantec_rtvscan.rb

  • Anonymous
    October 30, 2006
    Please note this kind of errors is catchable by new static analysis class of tools that are based on abstract interpretation techniques. The main benefit from those techniques is to eliminate the false negative. They also do reduce the number of false positive (not meaning eliminate). As a result, the underflow described in your example would have been highlighted before code review.

  • Anonymous
    October 30, 2006
    interesting defect!! thank you. Please note new static analysis engine based on abstract interpretation techniques can now find those issues. The main advantage of those techniques is that they remove false negative. As an example, the underflow would have been explicitely flagged as a underflow.

  • Anonymous
    October 31, 2006
    Even better, just use a safe function like strncat_s from the Safe CRT.  The buffer size is specified separately from "how many do you want to copy".  This way, you can use a simple (always correct) value for buffer size, and a possibly more complex expression for how-many. You can still get the wrong result (or crash), but at least you'll never have an overflow.

  • Anonymous
    November 14, 2006
    What is the new static analysis class or engine? Is it included in the Visual Studio 2005 or do you need to install it?