The Secure SDL: People->Process(Tool)

The Secure Development Lifecycle is primarily a people and process discipline.  The tools we implement in the Secure SDL, while important, are a tertiary concern.  What we mean is that you cannot solve the software security paradigm by throwing tools - and only tools - at the problem.  People must be trained, aware and performing well defined processes supported by well defined policy.

But that doesn't mean tools are not important.  You should acquire and implement appropriate tools in  your development lifecycle processes, such as IBM AppScan, Acunetix and Coverity, just to name a few.

Consider, for example, the linked list code that I previously posted.  When running the Xcode profiler, a null pointer dereference is detected.

Here's a snapshot of the output.  Take some time and understand what the tool is trying to communicate:

The question we must answer when reviewing the output of any SCA tool is this: is the tool reporting an actual vulnerability?  

Well, that depends.  

In my example the answer is yes: it has appropriately discovered a potential problem.  But is this possibly a false positive?  Let's find out.

The profiler picks up the problem here:

It's telling us that node is being assigned to head, which could be null, it presumes.  node is later used by Print(...) at which point a potential null pointer is dereferenced:

On the surface it's found a problem, and if you file a bug on this issue the developer may very well put some guards in place.  But if she's smart, she'll have a conversation with you first.  The question she should bring you is this: when is head null in this situation?  

To answer that question, you must dig deeper.  So, lets observe the declaration ... 

From there we learn that there may be some cause for concern, but we must also believe that the original developer seems to have assumed head wouldn't be null at the point flagged by the profiler, so where is it being initialized?  

The first place to look is the constructor.  We have one, LinkedList():

Bingo!  The code 'tail = head = new Node()' might return NULL.  We've found an allocation that's not being checked!  Check your pointers, we declare triumphantly.  But not so fast.  This is C++, not C.

The new method in C++ will throw an exception when memory cannot be allocated - unless we provide a callback for new, or disable the exception.  

Even so (to be on the safe side), we could refactor the constructor as follows:

After making the change, we execute the XCode profiler again only to find to our dismay, that the change did not reduce the profiler error.  

Since the constructor will always be called when creating the object, and there's seemingly no other way to instantiate this object without a valid head, then it appears our work is done here.  We'll just chalk the error up to a false positive.  

Ah, but no so fast, my pizza + caffeine == code friend.  

Recall that the overloaded Print(...) method in question is a public member.  What happens when the other method, Print(const Node*) const,  is called with a null pointer?  You'll crash a grizzly death.

Therefore, the correct fix to the problem is as follows:

So then, what have we demonstrated thus far?  A couple of things, at least.

First, coding problems are not always what they seem to be.  Tools can provide meaningful results, but their output must be interpreted correctly.  Just because a tool says you have a problem, doesn't mean you actually have a problem.

People - the developers and the security team - must be involved throughout the development lifecycle, performing the correct processes, such as running a static code analysis tool, logging appropriate defects and correcting problems.  

People->Process(Tool)