gearman / gearmand

http://gearman.org/
Other
741 stars 138 forks source link

libtest: fix potential unterminated string #267

Closed zvpunry closed 4 years ago

zvpunry commented 4 years ago

strncpy() may not terminate the dest if it is smaller than the given length.

This has been fixed to silence a compiler warning. It is just library test code, nothing that could cause problems in gearman.

esabol commented 4 years ago

I have confirmed that the build fails when compiled with gcc 9.2.1 + libmemached-dev + memcached installed on Ubuntu 18.04. With this change, the build succeeds.

For future reference, our usual fix of deliberately NUL-terminating the string right after the strncpy() call (something along the lines of memcached_binary_path[sizeof memcached_binary_path -1]= '\0';) doesn't work with gcc 9.2.1, even though it should. Something about the source string being arg_buffer.str().c_str() causes the check that gcc-9's -Wstringop-truncation does to fail. For example, this silences the warning:

      const char *foo = arg_buffer.str().c_str();
      strncpy(memcached_binary_path, foo, sizeof memcached_binary_path);
      memcached_binary_path[sizeof memcached_binary_path -1]= '\0';

but this doesn't:

      strncpy(memcached_binary_path, arg_buffer.str().c_str(), sizeof memcached_binary_path);
      memcached_binary_path[sizeof memcached_binary_path -1]= '\0';

Weird! Apparently, this is a bug in gcc 9.x according to https://stackoverflow.com/questions/56253996/why-does-gcc-9-1-0-sometimes-complain-about-this-use-of-strncpy/56255379 and is tracked at https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88780. Sadly not much progress on fixing that though, so we need to work around it. I prefer @zvpunry's solution here of just making the string one char bigger.