How can I mitigate integer overflow in this code?

Question

When I input "1073741824", it returns "Segmentation fault".

"1073741824" is 4294967296÷4, which is (INT_MAX+1)÷(sizeof(char *)).

and also, this is the malloc()'s parameter in this code.

But I don't know how to mitigate this problem.

Help me please.

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <limits.h>

int main(int argc, char **argv)
{
    int val, i;
    char *mem;

    if (argc < 2)
        exit(1);

    val = atoi(argv[1]);

   if (val > 0) {
        mem = malloc(val * sizeof(char *));

      if (mem == NULL) {
          printf("Failure\n");
          exit(2);
      }
    }

    for (i = 0; i < val; i++) {
        mem[i] = 'A';
        printf("%c", mem[i]);
    }

    printf("\n");

    return 0;
}

Do you have a choice of the compiler? If yes, you're better off with C++ try...catch for exception handling. — itsols
– itsols, Commented May 20, 2018 at 12:48
Where exactly does the segmentation fault occur? What are your debugging results? — Yunnosch
– Yunnosch, Commented May 20, 2018 at 12:49
You are only storing characters (1 or 2 bytes) in your allocated memory, why are you are allocating enough space to store character pointers (probably 4 or more bytes)? — Marker
– Marker, Commented May 20, 2018 at 12:52
I am just curious about the integer overflow. so I tested some codes, but I couldn't fix this by myself.. well,, I used gdb debugger, when I input 1073741824 as an argument but, I found the argument was 0x7fffffffe408 as a input in gdb. I don't know why. This is larger number than my input number. — Jay Cho
– Jay Cho, Commented May 20, 2018 at 14:33

Eric Postpischil · Accepted Answer · 2018-05-20 13:24:09Z

3

Likely, in your C implementation, int, size_t, and char * are each 32 bits, four bytes. When val is 1073741824, val * sizeof(char *) overflows and, as it happens, produces zero. Then mem = malloc(val * sizeof(char *)); allocates zero bytes of memory. It returns a valid pointer to zero bytes, not NULL, so your test for NULL does not cause your program to exit.

Then your program attempts to write 1073741824 bytes into the allocated memory. Since zero bytes were allocated, it overruns the space and crashes.

mem = malloc(val * sizeof(char *)); should be mem = malloc(val * sizeof(char)); or, better, mem = malloc(val * sizeof *mem):.

answered May 20, 2018 at 13:24

Eric Postpischil

232k15 gold badges197 silver badges377 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Jay Cho Over a year ago

Thanks a lot. This program printed "Failure" which means mem was null. Could you explain why "mem = malloc(val * sizeof * mem);" usage is more safe? I can't understand enough.

Eric Postpischil Over a year ago

@JayCho: Suppose you use mem = malloc(val * sizeof(char)) and, in the future, you want to change the code to use int instead of char. Then you have to change the definition of mem and the malloc. If you change one but not the other, the code will be broken. If you use mem = malloc(val * sizeof *mem);, then changing the definition of mem automatically changes the malloc. So the opportunity to create a bug is reduced.

Collectives™ on Stack Overflow

How can I mitigate integer overflow in this code?

1 Answer 1

2 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Related