1

I'm following Stanford CS155 security lesson's presentation to learn integer overflow. I learned today that memcpy() function may lead to overflow. The presentation says, If I have a code something like below, second memcpy() function may overflow heap. 

void  func( char *buf1, *buf2,    unsigned int len1, len2) {
  char temp[256];
  if (len1 + len2 > 256) {return -1}    // length check
  memcpy(temp, buf1, len1); // cat buffers
  memcpy(temp+len1, buf2, len2);
  do-something(temp);   // do stuff
}

How can I prevent overflow? What should I change in memcpy() so that it will not cause an overflow?

green
  • 11
  • 4
  • The questions title says *integer* overflow. I assume you meant *buffer* overflow, but wasn't sure so I didn't want to edit your question. You can [edit the question](https://security.stackexchange.com/posts/117316/edit) to correct. – Neil Smithline Mar 13 '16 at 18:18
  • Actually, the title in presentation was integer overflow, this is the reason that I wrote as integer not heap. I edit my question from integer to heap because it cause heap overflow. – green Mar 14 '16 at 07:43
  • Note that the question is not quite correct when it says the "second memcpy() function will overflow " : The second memcpy() _may_ overflow, or it may not - depends on the values of `len1` and `len2`. – sleske Mar 14 '16 at 10:26

1 Answers1

2

The problem is that you are adding the two integers before performing the length check.

if (len1 + len2 > 256) {return -1}    // length check

If len1 and/or len2 are sufficiently large, the addition will overflow, and the length check may go through even though the len1 and/or len2 are too large. To be safe, you must check twice:

if ((len1 > 256) || (len2 > 256) ||(len1 + len2 > 256)) {return -1}

For an example, try setting len1 and len2 to UINT_MAX/2+1. Then len1 + len2 will (usually) be 0.

sleske
  • 1,642
  • 12
  • 22
  • But, even if `len1` and `len2` is not bigger than 256, their summation result may bigger. Also as you said when `len1+len2` equal to 0, memcpy result with overflow, again. Am i wrong? – green Mar 16 '16 at 09:30
  • @green: Yes, even if len1 and len2 are not bigger than 256, their summation result may bigger - that's why checking the sum is still necessary (and is still checked in the code in my answer). The case `len1+len2 == 0` is covered by my answer - this can only happen because of an integer overflow in the addition, and that is prevented by checking `len1` and `len2` individually. – sleske Mar 16 '16 at 10:53
  • I'm sorry but, still don't see, which part of code is covered to the case of `len1 + len2 == 0` – green Mar 17 '16 at 11:28
  • @green: Sorry, I'm afraid we may have a language problem. The case `len1 + len2 == 0 ` is covered by the checks `len1 > 256` and `len2 > 256`. `len1 + len2 == 0 ` means that either `len1 == len2 == 0 ` (which is not a problem for the code), or there was an integer overflow, which is prevented by the checks `len1 > 256` and `len2 > 256` (there can be no overflow if both are smaller than 257, because the range of an `unsigned int` is guaranteed to be larger). If it's still not clear, consider asking a new question. – sleske Mar 17 '16 at 12:04