2
char imei_temp[14] = {0, };

strcpy(imei_temp, "00000000000000");

According to my understanding this is valid code.

But Klocwork is saying Buffer overflow, array index of 'imei_temp' may be out of bounds. Array 'imei_temp' of size 14 may use index value(s) 0..14

Levon
  • 138,105
  • 33
  • 200
  • 191
shunty
  • 375
  • 2
  • 7
  • 24
  • "size 14 may use index value(s) 0..14" -> very common incorrect assumption. Valid indices are 0..13. – Marlon Jun 21 '12 at 02:03

2 Answers2

13

It's a buffer overflow because your buffer is 14 bytes, but you are writing 15 bytes to it: 14 ascii "0"'s, and a null byte at the end.

Ned Batchelder
  • 364,293
  • 75
  • 561
  • 662
  • 1
    "To avoid overflows, the size of the array pointed by destination shall be long enough to contain the same C string as source (including the terminating null character), and should not overlap in memory with source." – Joe Jun 21 '12 at 00:44
3

when you specify a string using "s it adds an implicit \0 to the end of the string, you're trying to copy 15 bytes in to a 14 byte buffer.

Note, this doesn't happen when you specify a character using 's.

OmnipotentEntity
  • 16,531
  • 6
  • 62
  • 96
  • 1
    It's not specifying with `"`, it's `strcpy` appending the NUL byte which is the issue. – Asherah Jun 21 '12 at 00:56
  • `strcpy` does not append a NUL. `strcpy` will copy characters from a memory location until it finds a NUL. This means practically that if you specify a memory location that is not a string it is possible that a very large number of characters are copied until a NUL is found. But `strcpy` has no mechanism for appending a NUL. The `"`s are adding the NUL byte, you can prove this to yourself by dumping a string into a char array and `printf("%d", array[i])` in a for loop for the entire array. – OmnipotentEntity Jun 21 '12 at 01:00
  • Thanks for the answer, But as you said it will only copy 14 bytes, not the null terminated character. So the code should be valid. – shunty Jun 21 '12 at 01:11
  • 1
    @OmnipotentEntity: I never said that using `"` doesn't have append a NUL to the memory, but my point is that `strcpy` will copy *and include* the NUL. I think we're possibly saying the same thing in different ways. My point in saying "*It's not specifying with `"`*" is that it's `strcpy`'s behaviour which is ultimately responsible for 15 bytes being copied. (e.g. you could use `strncpy` and avoid this) – Asherah Jun 21 '12 at 01:13
  • 1
    @Len, `strcpy` will copy and include the NUL. `"` includes a NUL (if it didn't potentially many more bytes would be copied than just 14+1). We are saying the same thing, just talking past each other. @shunty, I did not say it would only copy 14 bytes. It will copy 15 bytes. All 14 `0`s and then a NUL. (`\0`) – OmnipotentEntity Jun 21 '12 at 01:19
  • @shunty "as you said it will only copy 14 bytes" -- No one said that, and it isn't true. I suggest reading the man page for strcpy before making claims about it. – Jim Balter Jun 21 '12 at 01:44
  • Len is right that specifying a string with `"` is irrelevant ... there are numerous other ways to provide a string consisting of 14 0's, and in every case there will be buffer overflow because C strings by definition are terminated with a NUL and strcpy copies it/appends it/whatever (those are semantically the same -- strcpy could copy every char until it sees a NUL and then append a NUL ... is it the "same" NUL or a different one? It's a meaningless distinction). – Jim Balter Jun 21 '12 at 01:50
  • Also, your second paragraph is meaningless and/or misleading -- a char literal has type char so it isn't big enough to also hold a NUL even if the compiler appended one. Likewise, note that in the case of `char ary[14] = "00000000000000";` no NUL is appended. – Jim Balter Jun 21 '12 at 01:54
  • Finally, compare Ned's answer, which is simple, direct, precise, and complete, covering every case regardless of how the string was specified, and not introducing anything extraneous. – Jim Balter Jun 21 '12 at 01:59
  • The reason why I specified the distinction between `"` and `'` is to point out the distinction between a `char` and a `string`. There are other ways to create a string without a trailing NUL. It's just the most common way. For your example, it would still work as expected even if you specified far too many characters for the array. It just generates a warning, which is suppressed if only the trailing NUL won't fit (in gcc at least.) Ned's answer is a good answer, I upvoted it. But please don't start a pissing match on stackoverflow. Who's honor are you defending? – OmnipotentEntity Jun 21 '12 at 02:09