Integer overflow while malloc memory

Question

We have struct image pointer:

struct image {
uint16_t size_x;
uint16_t size_y;
struct pixel *px;
};

and:

  img->px = malloc(sizeof(struct pixel) * height * width);

where pixel:

struct pixel {
   uint8_t red;
   uint8_t green;
   uint8_t blue;
   uint8_t alpha;
   }

where width & height:

  long height = strtol(height_arg, &end_ptr, 10);
  long width = strtol(width_arg, &end_ptr, 10);

So, since we use malloc() to allocate memory and it uses size_t for allocating. Since we multiplying height and width which are long typed to allocate memory, is there integer overflow expected? If, yes then how to handle it?

Later on we iterate over picture and color it:

  for (int i = 0; i < img->size_y; i++) {
      for (int j = 0; j < img->size_x; j++) {
         image_data[i][j].red = palette[0].red;
         image_data[i][j].green = palette[0].green;
         image_data[i][j].blue = palette[0].blue;
         image_data[i][j].alpha = 0xff;
      }
  }

where

 img->size_x = width;
 img->size_y = height;

The `sizeof` operator returns a `size_t` type; if that is `unsigned long long` (it may be), then the multiplication will be performed as `long long`. — Adrian Mole, Apr 12 '21 at 13:39
What possible values can you get for `width` and `height`? What are your requirements and limitations for the dimensions? — Some programmer dude, Apr 12 '21 at 13:39
@Someprogrammerdude There are no limitations since my goal is to find way to attack the program, find bug. And then fix it — Mark Ezberg, Apr 12 '21 at 13:41
On computers all integer types are limited. The limit might be very high (as is the case for 64 bit integers) but there still is a limit. It will always be possible to go over that limit if the code doesn't have checks for it. And the checks have to be from the user inputs. It doesn't matter if the input is read from a file, from a GUI, a text-based console, socket or any other way. If the input comes from outside the program it's "user" input that must be validated. — Some programmer dude, Apr 12 '21 at 13:43
@AdrianMole: In `unsigned long long * long long * long long`, the `long long` operands are converted to `unsigned long long`. Both multiplications are done as `unsigned long long`, not `long long`. — Eric Postpischil, Apr 12 '21 at 13:47
@Eric Thanks for clarifying that. Integer conversions with mixed signed/unsigned operands still give me issues. — Adrian Mole, Apr 12 '21 at 13:49
@Devolus Multiplying two 16-bit values gives a 16-bit result, which could overflow leading to problems in further calculations using that result. — Some programmer dude, Apr 12 '21 at 13:49
@Someprogrammerdude: Re “On computers all integer types are limited”: Not on my Turing machine. It just prints a message on the console asking the operator to insert another tape. — Eric Postpischil, Apr 12 '21 at 13:56
@EricPostpischil Well there's still a limit... How much tape can be manufactured in a limited universe? :) — Some programmer dude, Apr 12 '21 at 13:58
Can any random integer arithmetic expression in C overflow? Yes, it can. Why we need to be aware of the maximum values of every variable type involved, every single time we write an expression. — Lundin, Apr 12 '21 at 15:05

Adalcar · Answer 1 · 2021-04-12T13:55:18.417

2

Integer overflow may happen at every point of this program:

strtol("9999999999", &end_ptr, 10) -> will overflow and return LONG_MAX
height * width -> can overflow and return a negative value (since those are signed longs)
sizeof(struct pixel) * height * width 
-> can return a bigger value than UNSIGNED_MAX, wrapping around to a smaller value than expected

Malloc can allocate some very large data segments, and may not fail immediately, so you should seriously consider verifying your results before allocating.

edited Apr 12 '21 at 13:55

answered Apr 12 '21 at 13:47

Adalcar

1,458
11
26

Unless `size_t` is narrower than the size of `height` and `width`, `sizeof(struct pixel) * height * width` cannot overflow in the same way as `height * width` as `sizeof` is unsigned, so it differs in that the operation is defined to wrap (not overflow) and cannot return a negative value (since none is representable). – Eric Postpischil Apr 12 '21 at 13:49
@EricPostpischil Which is why I mentioned "returning a smaller value than expected". Wrapping around is still considered an overflow, no? Sorry if it was unclear – Adalcar Apr 12 '21 at 13:50
2

The C standard specifically uses “overflow” to mean the ideal result of an operation is not representable in the type, and it **defines** unsigned arithmetic to wrap, meaning the ideal result is the wrapped result. C 2018 6.2.5 9 explicitly states “A computation involving unsigned operands can never overflow,…” – Eric Postpischil Apr 12 '21 at 13:52
Ah, my bad, fixed the answer, thanks for the lesson – Adalcar Apr 12 '21 at 13:55

klutt · Answer 2 · 2021-04-12T14:11:12.217

2

Yes, there is a risk.

You can check it like this:

struct pixel *px;    

if (height > SIZE_MAX / width || sizeof *px  > SIZE_MAX / (width * height)) {
    // Handle overflow
} 

px = malloc(sizeof *px * width * height);

if(!px) {
    // Handle allocation error
}

img->px = px;

edited Apr 12 '21 at 14:11

answered Apr 12 '21 at 13:52

klutt

30,332
17
55
95

You also need to factor in the `sizeof(struct pointer)` – Emanuel P Apr 12 '21 at 13:53
@MarkEzberg You should also consider that `width` and `height` can also be 0 or negative. Also, you won't want to allocate `SIZE_MAX` amount of bytes. The best solution is to just predefine and check for a reasonable limit to the size of an image, and this should be less than any potential overflow (which is technically a wrapping around of the value, not an overflow, if you want to be pedantic) – Emanuel P Apr 12 '21 at 15:38

Integer overflow while malloc memory

2 Answers2