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


##########
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:
   Well the `strlen()` call would certainly have undefined behavior (including 
probable segfault) if the input string isn't NULL-terminated. And given the 
documented requirement that the input must be NULL-terminated, it seems like 
the options are:
   
   1) Provide invalid non-NULL-terminated input and get undefined results / 
possible segfault
   2) Provide valid NULL-terminated input and receive a NULL-terminated result.



-- 
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: dev-unsubscr...@guacamole.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to