Skip to main content

Overcoming Buffer Overflows: A real world case study

February 16, 2012

I recently performed a manual source code review of an application module written in C. The initial code base was riddled with buffer overflow vulnerabilities. There were over 1,000 instances of calls to strcpy, strcat, sprintf, gets and a few other “unsafe” functions. Needless to say, the client’s security report was quite lengthy. Their developers went back to the drawing board. Several weeks later, they delivered a refreshed code base.

I was amused to see a new function definition: pcicopy. My Spidey sense immediately started tingling. With a name like that, it had to be secure, right? Here is something similar to what they provided:

void pcicopy(char a[],char b[]) {    int tam,x;    tam=strlen(b);    for(x=0;x<tam;x++)       a[x]=b[x];    a[x]=0x00; }

Were they trying to fool anyone? By implementing their own version of strcpy, they again failed to consider the situation when b (the source buffer) would not be properly null-terminated. In cases like this, strlen returns undefined and unpredictable results. In addition, if array b contains more characters than is allocated to array a, a buffer overrun is still very much possible. Even though I wanted to say "nice try, but no" in the report, I stopped myself. Instead, I included further explanation of the flaw, with code samples, links to OWASP materials, and the de facto "use safe string functions such as strncpy and strncat" recommendation. All seemed right with the world, at least for a short while. Several weeks later, I received the next code release, which thankfully included fixes to most issues. Calls to gets were replaced with fgets, and no dangerous calls to strcpy and strcat could be found. Even pcicopy was reworked, and a new function called pcicat was added.

void pcicopy(char a[],char b[]) { int x=-1; do { x=x+1; a[x]=b[x]; }while(b[x]!=0x00 && x<1500); }

void pcicat(char a[],char b[]) { int x=-1,y=-1; do { x=x+1; }while(b[x]!=0x00 && x<1500); do { y=y+1; a[x]=b[y]; x=x+1; }while(b[y]!=0x00 && x<1500 && y<1500); }

OK, I thought, are they just messing with me now? As you can see, the developer hardcoded a length of 1500 for both functions. Unfortunately, pcicopy and pcicat are used for buffers of all sizes, from 20 to 1500 chars. This time, the report included firm recommendations that all buffer sizes should be checked and enforced by passing the size of the destination array as a parameter to the pcicopy and pcicat functions. I even included sample code that would fix the problem once and for all. However, the next round review brought even more cleverness.

void pcicopy(char dst[],char src[]) { dst[0]=0x00; if(strlen(src)<=sizeof(dst)) strncpy(dst,src,strlen(dst); else printf("\nError Buffer\n"); }

void pcicat(char dst[],char src[]) { if(strlen(dst)+strlen(src)<=sizeof(dst)) strncat(dst,(char *)&src[0],strlen(src); else printf("\nError Buffer\n"); }

Yikes! Though the developers now chose to indeed use the "safer" functions strncpy and strncat, they not only didn't fix the problem, but severely broke their code! In the case of pcicopy, strlen still doesn't account for a src string that isn't null terminated. Secondly, it calls sizeof(dst) which returns the size of the pointer to the char array, not the size of the array itself. Finally, even if strncpy is ever called (only for very short src strings), it doesn't copy anything, as strlen(dst) always returns 0 (dst[0]=0x00)! The pcicat function contains similar issues. How many rounds were we up to? At this point I lost count. Luckily, the next code update brought the following changes, which were finally an improvement.

void pcicopy(char dst[],char src[],int max_leng) { dst[0]=0x00; if(strlen(src)<=max_leng) strncpy(dst,src,strlen(dst); else printf("\nError Buffer\n"); }

void pcicat(char dst[],char src[],int max_leng) { if(strlen(dst)+strlen(src)<=max_leng) strncat(dst,(char *)&src[0],strlen(src); else printf("\nError Buffer\n"); }

Calls to both functions used sizeof(dst)-1 for the third parameter, and all arrays were either properly initialized or explicitly null-terminated. But what's this? The pcicopy still fails to copy anything to the destination array because strlen(dst) still resolves to 0. Oh, so close!

In the end, the code was fixed, but it left me wondering why it was so hard for these C developers to get it? Perhaps they weren't trained or required to unit test their code. Perhaps their non-Windows development environment or IDE was too difficult to work with. Maybe it was a language issue (they were a non-US development team). I thought my reports were clear enough when I practically said "do this and we can get on with life", but maybe not. I suppose it could have been some combination of these.

In any case, the whole experience brought a whole new appreciation for just how hard it can be to fix buffer overflow vulnerabilities. I hope you enjoyed! Shawn Asmus, CISSP, OSCP, is a member of FishNet Security’s application security team. Shawn conducts manual and tool-assisted code reviews, application penetration tests, and various other security aerobics.

    Shawn Asmus

By: Shawn Asmus

Practice Manager, Application Security, CISSP, CCSP, OSCP

See More

Related Blogs

June 07, 2018

Quick Tips for Building an Effective AppSec Program – Part 3

This is the last post in my series on creating an effective AppSec program within your organization. In my last post, we discussed the importance of t...

See Details

May 10, 2018

Observations on Smoke Tests – Part 3

While attending one of our technology partner’s security training courses, the instructor presented on their product’s various features and capabiliti...

See Details

May 03, 2018

Getting Started with Postman for API Security Testing: Part 1

Postman is a useful tool used by many developers to document, test and interact with Application Programming Interfaces (APIs). With the ubiquity of A...

See Details

How Can We Help?

Let us know what you need, and we will have an Optiv professional contact you shortly.

Privacy Policy


May 09, 2018

Application Security

Learn how Optiv can help protect your most critical enterprise applications from both internal and external threats.

See Details

July 15, 2014

Application Security by Obscurity | Optiv

“Security by obscurity” is a pejorative term to most in the security industry and with good reason. Typically, it’s just a matter of time before light...

See Details

Stay in the Know

For all the latest cybersecurity and Optiv news, subscribe to our blog and connect with us on Social.


Join our Email List

We take your privacy seriously and promise never to share your email with anyone.

Stay Connected

Find cyber security Events in your area.