necouchman commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595990647


##########
src/libguac/string.c:
##########
@@ -124,18 +124,30 @@ char* guac_strdup(const char* str) {
     /* Do not attempt to duplicate if the length is somehow magically so
      * obscenely large that it will not be possible to add a null terminator */
     size_t length;
-    if (guac_mem_ckd_add(&length, strlen(str), 1))
+    size_t length_to_copy = strnlen(str, n);
+    if (guac_mem_ckd_add(&length, length_to_copy, 1))
         return NULL;
 
-    /* Otherwise just copy to a new string in same manner as strdup() */
-    void* new_str = guac_mem_alloc(length);
-    if (new_str != NULL)
-        memcpy(new_str, str, length);
+    /* Otherwise just copy to a new string in same manner as strndup() */
+    char* new_str = (char*)guac_mem_alloc(length);
+    if (new_str != NULL) {
+        memcpy(new_str, str, length_to_copy);
+        new_str[length_to_copy] = '\0';

Review Comment:
   Okay, so that means that adding this explicit addition of a null terminator 
to the new string probably doesn't matter much one way or the other - it's the 
"safest" option in the case that a string happens to be passed in that doesn't 
have a null terminator and gets through the various steps (`strlen` in 
particular) without causing issues.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to