]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commitdiff
Fix buffer overflow in Str_Strncat() found by Coverity.
authorOliver Kurth <okurth@vmware.com>
Fri, 15 Sep 2017 18:22:45 +0000 (11:22 -0700)
committerOliver Kurth <okurth@vmware.com>
Fri, 15 Sep 2017 18:22:45 +0000 (11:22 -0700)
open-vm-tools/lib/string/str.c

index 6e65833a853ecd5f12a080d7bfb22044c5adb88e..8568414c92032823de17c37ac2db07609b79d396 100644 (file)
@@ -456,21 +456,18 @@ Str_Strncat(char *buf,       // IN/OUT
    ASSERT(src != NULL);
 
    /*
-    * Check bufLen + n first so we can avoid the second call to strlen
-    * if possible.
+    * If bufLen + n is OK, we know things will fit (and avoids a strlen(src)).
+    * If bufLen + n looks too big, they still might fit if the src is short
+    * enough... so repeat the check using strlen(src).
+    *
+    * The "fit" means less than, as strncat always adds a terminating NUL.
     *
-    * The reason the test with bufLen and n is >= rather than just >
-    * is that strncat always NUL-terminates the resulting string, even
-    * if it reaches the length limit n. This means that if it happens that
-    * bufLen + n == bufSize, strncat will write a NUL terminator that
-    * is outside of the buffer. Therefore, we make sure this does not
-    * happen by adding the == case to the Panic test.
     */
 
    bufLen = strlen(buf);
 
-   if (bufLen + n >= bufSize &&
-       bufLen + strlen(src) >= bufSize) {
+   if (!(bufLen + n < bufSize ||
+         bufLen + strlen(src) < bufSize)) {
       Panic("%s:%d Buffer too small\n", __FILE__,__LINE__);
    }