Re: [libvirt] [PATCH] Fix latent buffer overflow in qemudOpenMonitorUnix.

Chris Lalancette wrote: Sigh. Unfortunately, this is a difficult problem. I did some reading, and found that besides having the NULL termination problem, strncpy() is also very slow, so should be avoided. I then looked at doing an implementation of strlcpy(), but while it would prevent the buffer overflow, it can truncate the resulting string. This could lead to difficult detect errors later on. Note that both my original proposed patch for this problem plus many of current in-tree users of strncpy() suffer from this problem as well. Therefore, I think the way to go is probably to have a safe version of strcpy() which checks that the length of destination is sufficient to hold all of the source plus the \0. If not, it should return an error, and then the caller can decide what to do. Something like this: /** * virStrcpy * * A safe version of strcpy. The last parameter is the number of bytes * available in the destination string, *not* the number of bytes you want * to copy. If the destination is not large enough to hold all of the * src string, NULL is returned and no data is copied. If the destination * is large enough to hold all of the src, then the string is copied and * a pointer to the destination string is returned. */ char * virStrcpy(char *dest, const char *src, size_t destbytes) { int len; len = strlen(src) + 1; if (len > destbytes) return NULL; return memcpy(dest, src, len); } And usage would be: char foo[5]; virStrcpy(foo, "hello", sizeof(foo)); /* returns an error, foo is not large enough for hello\0 */ char bar[6]; virStrcpy(bar, "hello", sizeof(foo)); /* succeeds, foo is large enough for hello\0 */ A brief look through the current usage of strncpy() in libvirt shows that this would be a drop in replacement for most of the current users of strncpy(), and be safer as well. What do you think? -- Chris Lalancette