What was the most dangerous programming mistake you have made in C?

Question

I am an intermediate C programmer. If you have made any coding mistake that you came to know later that it was the most hazardous / harmful to the total application please share that code or description. I want to know this because in future I may come across such situations and I want to have your advice to avoid such mistakes.

Answer 1

if (c = 1) // insert code here

Answer 2

for(int i = 0; i<10; ++i)
  //code here
  //code added later

Note that the later added code is not in the for loop.

Answer 3

One thing to look after are array bounds. If you go out of bounds, with bad luck you may end up overwriting memory that is used for other data.

One nasty bug related to this was going out of bounds for a static array variable in a function. That ended up as a function changing values of the local variables of the calling function. That wasn't so straight-forward to debug..

Answer 4

Few years ago I've got a call from my ex-colleague telling me about the problem he had to fix with my code, which was a router for credit card transactions.

Card number prefix consists of 6-digit BIN (Bank Identification Number) and extra few digits that banks use at own discretion, e.g. bank has BIN for Visa Classic card 456789, and reserve 2 extra digits to indicate sub-product, like 01 for student's card, 02 for co-branded card with local department store and so on. In this case card prefix, which is basically product identifier, becomes 8 digits long. When I coded this part, I decided that 9 digits "ought to be enough for everyone". I was running ok for 2 years until one day bank make a new card products with 10-digit-long prefix (have no idea why they needed it). Not too hard to imagine what has happened - router segfaulted, the whole system halted because it cannot function without transaction router, all ATMs of that bank (one of biggest in the country) became non-operational for few hours, until problem was found and fixed.

I cannot post the code here firstly because I don't have it and secondly it is copyrighted by the company, but it is not hard to imagine the strcpy() without checking size of target buffer.

Just like man strcpy says:

If the destination string of a strcpy() is not large enough (that is, if the programmer was stupid or lazy, and failed to check the size before copying) then anything might happen. Overflowing fixed length strings is a favorite cracker technique.

I was very embarrassed. It was a good time to commit seppuku :)

But I learned the lesson well and do not forget (usually :) ) to check size of target buffer. I wouldn't recommend you to learn it the hard way - just develop a habit to check target buffer before strcpy() and strcat().

Edit: good suggestion from Healthcarel - use strncpy() rather than strcpy(). It doesn't add trailing 0 but I usually use following macro to get around it:

#define STRNCPY(A,B,C) do {strncpy(A,B,C); A[C] = 0; } while (0)

Answer 5

It has been a long time, but some things you never forget ;-).

• forget the \0 at the end of a string.
• allocate n characters for a string with n characters.
• forgetting the break in a switch statement.
• 'creative' macro use.

Answer 6

Using non-limited string functions such as strcpy() or strcmp(), instead of safe versions like strncpy() and strncmp().

Answer 7

Passing virtual address to the DMA engine was a worst one, not exactly C related, but i assume that 99% of DMA related stuff written in C so it's kind of match. This small error lead to memory corruption that took me 1.5 month to find.

Answer 8

You should worry more about little mistakes. Big/spectacular mistakes are usually documented in books (with reasons why they are bad, alternative approaches etc.).

It's the little design/coding mistakes that get to you, because they tend to add up.

So my advice is to try and read the books Kernighan wrote or co-authored ("The C Programming Language", "Practice of Programming" etc.) because they are full of common-sense (common for experienced C programmers) advice and list principles that are very useful in avoiding both small and big mistakes.

They also list many potential big mistakes, so they answer your initial question.

Answer 9

if(a == true);
{
  //Do sth when it is true. But it is allways executed.
}

Edit: Another variant of the same mistake.

for(i=0; i<max_iterations;i++);
{
  //Do sth but unexpectedly only once
}

Answer 10

The most dangerous thing I ever did in C was trying to write code which managed my own memory. Effectively, this means the most dangerous thing I ever did in C was write C code. (I hear that you can get around it these days. Hip hip for sanity. Use those approaches whenever appropriate!)

• I don't write paging algorithms -- OS geeks do that for me.
• I don't write database caching schemes -- database geeks do that for me.
• I don't build L2 processor caches -- hardware geeks do that for me.

And I do not manage memory.

Someone else manages my memory for me -- someone who can design better than I can, and test better than I can, and code better than I can, and patch when they make critical security-compromising mistakes which only get noticed 10 years later because absolutely everyone who attempts to allocate memory fails some of the time.

Answer 11

I take the definition of dangerous as "we may ship with that bug and discover only years later when it's to late":

char* c = malloc(...);
.
.
.
free(c);  
.
.
.
c[...] = ...;

or

// char* s is an input string
char* c = malloc(strlen(s));
strcpy(c, s);

But if you write multiplatform (not limited to x86/x64) this is also great:

char* c = ...;
int i = *((int*)c); // <-- alignment fault

And if your buffer comes from an untrusted source.. basically most code around is dangerous.

But, anyway, in C it's so easy to shoot yourself in the foot, that a topic about shot feet could go around the thousands of pages.

Answer 12

while(a)
{ //code - where 'a' never reaches 0 :( }

Answer 13

switch case with no break.

Answer 14

I remember two blunders:

1. returning address of an auto variable from within a function in which it was created;
2. copying string to an uninitialised and unallocated pointer to char.

Answer 15

Uninitialized data.

Answer 16

system() with some user-supplied string in the argument. Same goes for popen().

Use exec*() instead.

Of course, this is not unique to C.

Answer 17

I'm in agreement with Pat Mac here (despite his downvoting). The most dangerous thing you can do in C is simply to use it for something important.

For example, a sensible language would by default check array bounds and stop your program immediately (raise an exception or something) if you try to wander outside of it. Ada does this. Java does this. Tons of other languages do this. Not C. There are entire hacking industries built around this flaw in the language.

One personal experience with this. I worked with a company that ran a network of flight simulators, tied together with reflective (shared) memory hardware. They were having a nasty crash bug they couldn't track down, so our two best engineers got sent up there to track it down. It took them 2 months.

It turned out that there was an off-by-one error in a C loop on one of the machines. A competent language would have stopped things right there of course, but C let it go ahead and write a piece of data in the next location past the end of the array. This memory location happened to be used by another machine on the network which passed it to a third machine, which used the (garbage) value as an array index. Since this system was also coded in C, it didn't care either that it was indexing way outside of its array, and trashing semi-random memory locations in its program.

So, due to a lack of array bounds checking, a simple easy to make off-by-one bug caused random crashes in a computer two entire hops away from the source of the bug! Cost to the company: 4 man-months of their best engineers' time, plus however much was spent by other engineers and support personnel, plus all the downtime from the simulators in question not working right.

Answer 18

#include <string>

Me thinking C supports string natively (using Metroworks codewarrior, about 8 years ago).

I did this, for a final project of about 15,000 lines of code. I used this library to do everything relating to strings(appending, splitting, etc.) Only to have the TA's not able to compile any bit of my assignment(using GCC.)

Little did i learn that metroworks had created their own string library. I failed that class.

Answer 19

Two things comes to mind. Ths first was a function in embedded C (MCU) i tried to have some restrictions on an timer value as a entered a function. so I wrote

if(55000 < my_var < 65000)

My ida was to check like this:

if( (55000<my_var) < 65000)

But this is the equivilant or the result

if( (55000<my_var) || (my_var<65000))

and the result ended up that the if test was always true.

The secound was a pointer mistake. (simplyfied presented here)

get_data(BYTE **dataptr)
{ 
  ubyte* data = malloc(10);
  ... code ...
  *dataptr = &data[1];
}

 main()
 {
   BYTE *data
   get_data(&data);
   free(data);
 }

Thus resulting in a loss of 1 byte of memory for each time the get_data() function was called

Answer 20

if (importantvar = importantfunction() == VALID_CODE)

This is when I meant this:

if ((important var = importantfunction()) == VALID_CODE)

This led to many hours of debugging troubles when I assumed it worked like the latter.

Answer 21

Having been a Lisp programmer, I was used to indenting closing braces, as in:

(cond
    ((eq a foo)(bar ...
        ....
        ))
    )

and I carried this into C programming:

if (a == foo){
    bar(...);
    ....
    }

Then I got on a good-size project in C, and another programmer had to make changes in the vicinity of my code. He mis-read my closing brackets, and freed some memory too early. This caused an extremely subtle bug that happened at crunch time. When it was found, he got blamed, badly. But you could say it was my fault. That was not fun, to say the least.

Answer 22

Forgetting architecture constraints and happily memcpy()ing into the memory mapped I/O area on a microcontroller. The magic smoke was released from the test rig.

Answer 23

I was dealing with dynamically allocated 2D arrays and I instead of free()'ing N rows, I decided to free M columns. This was fine for smaller inputs where N == M but on large inputs, I only free()'d 50% of what I allocated.

shrug

Live and learn.