gasparfm / dwgo Goto Github PK

The manual page for strcat (3) explains

  The strcat() function appends the src string to the dest string, overwriting the
  terminating null byte ('\0') at the end of dest, and then adds a terminating null byte.

  The strings may not overlap, and the dest string must have enough space for the result.

  If dest is not large enough, program behavior is unpredictable; buffer overruns are 
  a favorite avenue for attacking secure programs.

The destination for strcat on lines 324 and 325 of dwgo.cpp are only suffcient for the strings being copied but not the additional terminating null byte.

dwgo.cpp:324:  other_dir=(char*)malloc(strlen(homedir)+6);  strcpy(other_dir, homedir);  
               strcat(other_dir, (char*)"/.dwgo");

dwgo.cpp:325:  datadir  =(char*)malloc(strlen(DATADIR)+5);  strcpy(datadir, DATADIR); 
               strcat(datadir, (char*)"/dwgo");

To accommodate the additional terminating null byte, each call to malloc needs an increase in size by one.

Line 324 should contain "+7" not "+6", viz the length of the string "/.dwgo" plus one for the terminator.

Line 325 should contain "+6" not "+5", viz the length of the string "/dwgo" plus one for the terminator.

Some may think that because the orginal strings for homedir and DATADIR, which are used in calculating the size of the destinations, contain a terminating null byte, that the length of the destination strings should be adequate without the "+1", but this overlooks the fact that (from the manual page of strlen (3)

    The strlen() function calculates the length of the string s,
    excluding the terminating null byte ('\0').