code review for buffer overflows

Question

3.00/5 (2 votes)

See more:

Hi people,

I need to do some review of code for buffer overflows.

I know usually we have insecure C functions e.g., strcpy, strcat, gets..
which should be probably removed if occurred in code.

Then I believe they must be replaced by their secure alternatives
right? strncpy, strncat, ... etc.

Is this the right way to do (security) code review?
Maybe there are some good links and resources on such guidelines, as to how this process should be done well?

thanks.

ps. Just discovered this tool: http://www.dwheeler.com/flawfinder/[^]
Is anyone familiar with it? Do you know if it is good?

Posted 29-Oct-12 20:28pm

WebMaster

Updated 29-Oct-12 22:38pm

v2

Add a Solution

Comments

Mohibur Rashid 30-Oct-12 4:46am

If you think strncpy is a secure version of strcpy then it is a bad idea. There is no way strncpy is secure at all. It has specific purpose. Here is an example of strncmp

<pre lang="c++">
char a[]={1, 2, 3, 4, 0,0,0,0};
char b[]={1, 2, 3, 4, 0,1,0,0};

int res=strncmp(a, b,6);
</pre>

Can you guess what you would get in res? if it was a secure version of strcmp then you would get 1 but in this case you would get 0. why? because even though the length says to compare upto 6 bytes. but it will stop on byte 5 because they are null terminated at byte 5

Barakat S. 30-Oct-12 8:37am

Don't trust or rely on automation tools. The rule is to not write an insecure code in the first place.

These functions are not "insecure", except for gets, but they can be used in an insecure way. For example, in this piece of code:

+-------------------------------

char buff[10];

if( (argv[1] == NULL) || (strlen(argv[1]) > 9) ) {
fprintf(stderr, "You must pass a string with lenght less than 10\n");
return -1;
}

strcpy(buff, argv[1]);

+-------------------------------

There is no need to use strncpy because you know that a string of length greater than 9 will never pass.

Instead of using tools, learn how to write a secure code. There are many articles there on the Internet on how to do it. I recommend reading through "CERT Secure Coding Standards"[1]. If you use gcc, activate all warnings by passing -Wall and take these warnings seriously. By the time, you will learn on your won.

[1] https://www.securecoding.cert.org/confluence/display/seccode/CERT+Secure+Coding+Standards

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

Eugen Podsypalnikov · Answer 1 · 2012-10-29T20:48:00

Solution 1

You could (make&)use C++ objects to manage the memory operations proper
or just take the Row data level[^] :)

Posted 29-Oct-12 20:48pm

Eugen Podsypalnikov

Member 2954534 · Answer 2 · 2012-10-29T21:52:00

Solution 2

As far as I know, the secure versions of strccpy, strcat etc. are strcpy_s, strcat_s and so on. strcpy, strncpy etc. generate a compiler warning telling you to use the "_s" - versions instead.

Posted 29-Oct-12 21:52pm

Member 2954534

Comments

[no name] 30-Oct-12 4:11am

I am doing this on Linux - I hope won't be a problem to use strcpy_s etc. functions? (sorry I'm not that familiar with such techniques - that is why I am asking - sorry if this is a newbie question).

Mohibur Rashid 30-Oct-12 4:29am

its visual studio issue :)

nv3 30-Oct-12 4:46am

There are many more issues to be covered when doing a code review for potential buffer overruns. It is not just strcpy and its brothers. This is something for an experienced C++ programmer and not a newbie.

[no name] 30-Oct-12 5:13am

I am not a newbie in the sense that I am a newbie programmer - just I never looked at these strcpy_s functions before so was not sure if they could be used under Linux.

Anyway, I came down to some static analysis tools - but I am looking for "modern" static analysis tools - do you know which are they?? e.g., such as Flawfinder (which is a bit old, but I think it is good one or RATS - it seems RATS is not supported anymore?). thanks

nv3 30-Oct-12 5:33am

I did not want imply that you are a newbie, just that it takes an experienced C++ programmer to do a useful code review of any kind. Look at your recent questions and decide for yourself if your experience level matches that task.

As for tools: There are dozens of tools on the market, all with their specific strengths. I can not really give a preference for any of them as we put more emphasis at looking at the code by humans than doing routine checks with analysis tools.

[no name] 30-Oct-12 6:30am

Ok - got it. In any case I don't thin I am a newbie - I have several years of programming experience. Some my questions were asked since I was in rush and was not able to find proper advice on internet, or were related to some specific C++ Details. Anyway, using a tool is a good idea - because it can spot errors which maybe missed by humans + it does no harm to use a tool too - only it can be a plus.