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