Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67870 )
Change subject: helpers.c: Fix undefined behavior in strndup() ......................................................................
helpers.c: Fix undefined behavior in strndup()
Using strlen() or strdup() inside strndup() is problematic: if the input string is not null-terminated, these functions can read past the end of the buffer, which triggers undefined behavior. Rewrite the function to never read past the provided `maxlen` bound.
Change-Id: Id34127024085879228626fbad59af03268ec5255 Signed-off-by: Xiang Wang merle@hardenedliux.org Reviewed-on: https://review.coreboot.org/c/flashrom/+/49741 Reviewed-by: Angel Pons th3fanbus@gmail.com Reviewed-by: Edward O'Callaghan quasisec@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-on: https://review.coreboot.org/c/flashrom/+/67870 Reviewed-by: Felix Singer felixsinger@posteo.net --- M helpers.c 1 file changed, 30 insertions(+), 8 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved
diff --git a/helpers.c b/helpers.c index c83cd2c..289848d 100644 --- a/helpers.c +++ b/helpers.c @@ -106,15 +106,16 @@ /* strndup is a POSIX function not present in MinGW */ char *strndup(const char *src, size_t maxlen) { - if (strlen(src) > maxlen) { - char *retbuf; - if ((retbuf = malloc(1 + maxlen)) != NULL) { - memcpy(retbuf, src, maxlen); - retbuf[maxlen] = '\0'; - } - return retbuf; + char *retbuf; + size_t len; + for (len = 0; len < maxlen; len++) + if (src[len] == '\0') + break; + if ((retbuf = malloc(1 + len)) != NULL) { + memcpy(retbuf, src, len); + retbuf[len] = '\0'; } - return strdup(src); + return retbuf; } #endif