necouchman commented on code in PR #517:
URL: https://github.com/apache/guacamole-server/pull/517#discussion_r1595849497
##########
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:
> Looks to me like guac_strdup() did add a NULL terminator... it added 1 to
strlen(str) and then copied that many bytes over, which will include the
terminator. What am I missing?
I would agree that it copies over the NULL terminator if the original string
has one, but it does not manually add one to the end of the string. But maybe
I'm missing something?? Here's the current source code:
https://github.com/apache/guacamole-server/blob/3ad3b041f273ec1ed30e49be49f1adcf0edd0b61/src/libguac/string.c#L118-L137
It looks to me like it:
* Checks if `str` is `NULL`, returning `NULL` if that is the case.
* Checks the string length, returning `NULL` if it cannot fit in memory.
* Allocates the memory, and then uses `memcpy` to copy over the memory.
* Returns the string.
I don't see any explicit addition of a null terminator in that function,
whereas the modified version, here, does explicitly add a null terminator, as
does the `guac_strlcpy()` function earlier in the file.
--
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]
