Avoiding C++ vulnerabilities

Just returned from Blackhat – it always seems that the presentations I most want to see happen at the same time as I'm scheduled to talk. Neel Mehta, John McDonald and Mark Dowd were talking about finding exploitable C++ specific flaws, and I was only able to watch the first 30 minutes. Something came up there that is addressed by a technique documented by Scott Meyers in "Effective C++". Here's the problem – if ownership of some resource can be ambiguous, all sorts of bugs (security and otherwise) can happen. This happens because if I declare a class like so:

class Foo
{
public:
int j;
};

There are actually 4 operators created by default:

• Constructor - void Foo()
• Destructor - void ~Foo()
• Copy constructor - void Foo( const Foo& )
• Assignment operator - const Foo& operator=( const Foo& )

The last two are what gets us in trouble, especially considering that most programmers don't know these get created. Where this gets us in trouble from a security standpoint is when I go and make a class like so:

class DumbPointer
{
public:
void DumbPointer() : m_ptr( 0 ){}
void ~DumbPointer() { if( m_ptr ) free( m_ptr ); }
// other stuff left out
private:
void* m_ptr;
};

What happens here is that when I assign this to another DumbPointer object, no one is really sure when the pointer gets freed. This leads to a lot of badness, and can happen in odd places, like passing an instance of the class by value, which will invoke the copy constructor. Unless you're willing to put in the work needed to transfer ownership, or do ref counting, the best thing to do is to explicitly tell everyone not to do this like so:

private:
void DumbPointer( const DumbPointer& ); // don't implement this
const DumbPointer& operator=( const DumbPointer& ); // or this one, either

What now happens is that if someone does inadvertently invoke one of these operators, they won't compile, and if you invoke one of them internal to your class, it won't link. The resulting compiler errors are less than intuitive, but it's better than undefined behavior.

BTW, this is items 5 and 6 in "Effective C++".

Comments

• Anonymous
  August 04, 2007
  Wow - this was presented as new security research? Every C++ coder should know this stuff already. Or were they deliberately doing an overview of C++ coding issues before delving into new stuff? [dcl] Almost all of the information around code review and finding problems is oriented towards C. While every C++ programmer should know this, in practice, not many do. I think they're trying to educate people who do security review work about C++-specific issues. However, as I said to start with, I only got to see 1/3 of their talk, so I likely missed the good stuff.

• Anonymous
  August 10, 2007
  IIRC, the correct signature for an assignment operator does not allow the initial "const" -- it should be: DumbPointer & operator = ( const DumbPointer & ); I remember winning a bet over that one before.  IIRC, it's because if you make the return value "const", it breaks chaining assignments: p0 = p1 = p2; [dcl] Whups - you're correct on this. Just checked Meyers as a reference, and you're right.

• Anonymous
  August 13, 2007
  Re: Aaron Actually having the return value const does not break chaining assignments, because the result of the first (p1 = p2) is the rhs of the next assignment, so it being const or not changes nothing in this regard. The difference appears only when somebody writes (p0 = p1) = p2; that is, assigns p1 to p0 and immediately afterwards assigns p2 to p0, which is completely pointless. Meyers states in his book that even though there is no reason to ever do such a thing the standard makes it possible for built-in types, so he wants to make it possible for the classes he writes as well and therefore makes the return value of operator= non-const. My opinion on this diverges: I consider that since writing this is pointless, if anybody does it it can only be a mistake; so I want the compiler to catch this, and therefore I make the return value of my assignment operators const. It's the same idea as with making copy constructors private: be helpful to the users of the class by being as restrictive as possible, to allow the compiler to catch as many potential errors as possible. [dcl] OK - great feedback. I always like it when I mistakenly do something even better than usual 8-)

• Anonymous
  April 21, 2008
  Seems my last post met with some objections – somewhat rightfully so, as I mischaracterized one of Tom's