From b6b27d3726ad0dcf6030dbfe2baf56e57ae490c7 Mon Sep 17 00:00:00 2001 From: Tobias Stoeckmann Date: Fri, 8 Nov 2024 17:08:41 +0100 Subject: [PATCH] libkmod: Fix overflow in kmod_module_hex_to_str If an overly long signature is found in a module file, it is possible to trigger an out of boundary write in kmod_module_hex_to_str due to integer and subsequent heap buffer overflow. This approach replaces malloc + sprintf with a simple hex-lookup and a strbuf approach, being slightly faster in real life scenarios while adding around 100 bytes to library size. A much faster approach could be done without strbuf and using our overflow check functions, but readability should win here. Signed-off-by: Tobias Stoeckmann Link: https://github.com/kmod-project/kmod/pull/236 Signed-off-by: Lucas De Marchi --- libkmod/libkmod-module.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c index cdf90135..eb5861cf 100644 --- a/libkmod/libkmod-module.c +++ b/libkmod/libkmod-module.c @@ -22,6 +22,7 @@ #include #include +#include #include #include "libkmod.h" @@ -1798,29 +1799,29 @@ static struct kmod_list *kmod_module_info_append(struct kmod_list **list, const static char *kmod_module_hex_to_str(const char *hex, size_t len) { - char *str; - int i; - int j; + static const char digits[] = "0123456789ABCDEF"; + struct strbuf sbuf; const size_t line_limit = 20; - size_t str_len; - str_len = len * 3; /* XX: or XX\0 */ - str_len += ((str_len + line_limit - 1) / line_limit - 1) * 3; /* \n\t\t */ + strbuf_init(&sbuf); - str = malloc(str_len); - if (str == NULL) - return NULL; - - for (i = 0, j = 0; i < (int)len; i++) { - j += sprintf(str + j, "%02X", (unsigned char)hex[i]); - if (i < (int)len - 1) { - str[j++] = ':'; + for (size_t i = 0; i < len; i++) { + if (!strbuf_pushchar(&sbuf, digits[(hex[i] >> 4) & 0x0F]) || + !strbuf_pushchar(&sbuf, digits[hex[i] & 0x0F])) + goto fail; + if (i < len - 1) { + if (!strbuf_pushchar(&sbuf, ':')) + goto fail; - if ((i + 1) % line_limit == 0) - j += sprintf(str + j, "\n\t\t"); + if ((i + 1) % line_limit == 0 && + !strbuf_pushchars(&sbuf, "\n\t\t")) + goto fail; } } - return str; + return strbuf_steal(&sbuf); +fail: + strbuf_release(&sbuf); + return NULL; } static struct kmod_list *kmod_module_info_append_hex(struct kmod_list **list,