Is this code vulnerable to buffer overflow?

Question

Fortify reported a buffer overflow vulnerability in below code citing following reason -

In this case we are primarily concerned with the case "Depends upon properties of the data that are enforced outside of the immediate scope of the code.", because we cannot verify the safety of the operation performed by memcpy() in abc.cpp

void create_dir(const char *sys_tmp_dir,  const char *base_name,
                size_t     base_name_len)
{
    char        *tmp_dir;
    size_t      sys_tmp_dir_len;

    sys_tmp_dir_len = strlen(sys_tmp_dir);

   tmp_dir = (char*) malloc(sys_tmp_dir_len + 1 + base_name_len + 1);
   if(NULL == tmp_dir) 
     return;

memcpy(tmp_dir, sys_tmp_dir, sys_tmp_dir_len);
tmp_dir[sys_tmp_dir_len] = FN_LIBCHAR;
memcpy(tmp_dir + sys_tmp_dir_len + 1, base_name, base_name_len);
tmp_dir[sys_tmp_dir_len + base_name_len + 1] = '\0';
..........
..........

}

It appears to me a false positive since we are getting the size of data first, allocating that much amount of space, then calling memcpy with size to copy. But I am looking for good reasons to convince fellow developer to get rid of current implementation and rather use c++ strings. This issue has been assigned to him. He just sees this a false positive so doesn't want to change anything.

Edit I see quick, valid criticism of the current code. Hopefully, I'll be able to convince him now. Otherwise, I'll hold the baton. :)

Answer 1

Take a look to strlen(), it has input string but it has not an upper bound then it'll go on searching until it founds \0. It's a vulnerability because you'll perform memcpy() trusting its result (if it won't crash because of access violation while searching). Imagine:

create_dir((const char*)12345, baseDir, strlen(baseDir));

You tagged both C and C++...if you're using C++ then std::string will protect you from these issues.