Not null terminated string false alarm?

Question

Klocwork is producing an alarm which seems to be a false one. The bug it mentions describes about 80% of the total bugs in our code. Please advise,

Hereby is a snip set (paraphrase):-

//a snip set
// no bug here //

{
  char*     destStr;
  destStr = (char*)malloc(150);
  if (destStr != NULL) {
    destStr[0]= '\0';  //__here is the difference__ 
    char * myStr = malloc(200) ; 
    if (myStr != NULL) {
      strcpy(myStr , destStr) ; 
    }
    free(myStr);
  }
  free (destStr);
  destStr = NULL; 
}

//__whereas a bug here__ !

{
  char* destStr;
  destStr = (char*) malloc(150);
  if (destStr != NULL) {
    destStr[0]= '\0'; // __here is the difference__ 
  }
  else {
    printf("hello world \n");
  }
  if (destStr != NULL) {
    char * myStr = malloc(200); 
    if (myStr != NULL) {
      strcpy(myStr , destStr);   // __NNTS (not NULL terminated string) –  Buffer overflow of 'myStr' due to non null terminated string 'destStr'.__ 
    }
    free (myStr);
  }
  free (destStr);
  destStr = NULL; 
}
//end of snip set

Answer 1

What version of Klocwork's products are you using? I just tried analyzing the code sample as provided and got nothing reported. Adding an intentional NPD into the code did cause a report, just to prove I was actually running the tool ;p Suggest if you're not running something reasonably recent that you try upgrading (Insight 9.1 is the most recent released product set).

Regards, Gwyn Fisher CTO and VP R&D Klocwork, Inc gwyn-at-klocwork.com

Answer 2

Please paste formatted code (read Readable code)

At first i thought this is obfuscated in nature.

Wrt to the question, when you do a strcpy you need to check if the destination string is big enough to hold the source string.

Here DEST_LEN is equal to amount of memory in bytes allocated.

if(source != NULL && dest != NULL)

{

strncpy (dest , source , DEST_LEN -1);

}

---

Thanks for the editing by the moderator.

Klockworks detects strcpy as error as its just a static analysis tool. I would suggest you to define custom macros for string related operations. This will check the length of memory to be copied. For other operations also you can easily edit this macro and avoid FALSE ALARMS like above.