From 62f8363caa017741db8e5ced9c0f1e305f61c5ea Mon Sep 17 00:00:00 2001 From: Karel Zak Date: Tue, 7 Oct 2025 12:48:41 +0200 Subject: [PATCH] lib/mbsalign: use snprintf() instead of sprintf() Add bufsiz parameter to mbs_safe_encode_to_buffer() and mbs_invalid_encode_to_buffer() functions to enable safe buffer operations. Track remaining buffer size internally using int variable and convert all sprintf() calls to snprintf(). Add buffer overflow protection by checking snprintf() return values and verifying sufficient space before memcpy() and direct writes. Break encoding loop early if buffer space is exhausted. This change improves code safety by preventing potential buffer overflows and makes buffer size limits explicit in the API. Signed-off-by: Karel Zak --- include/mbsalign.h | 4 +-- lib/buffer.c | 2 +- lib/mbsalign.c | 68 +++++++++++++++++++++++++++++++--------- libsmartcols/src/print.c | 2 +- 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/include/mbsalign.h b/include/mbsalign.h index c1426105c1..4c388b36e8 100644 --- a/include/mbsalign.h +++ b/include/mbsalign.h @@ -53,11 +53,11 @@ extern size_t mbs_nwidth(const char *buf, size_t bufsz); extern size_t mbs_width(const char *s); extern char *mbs_safe_encode(const char *s, size_t *width); -extern char *mbs_safe_encode_to_buffer(const char *s, size_t *width, char *buf, const char *safechars); +extern char *mbs_safe_encode_to_buffer(const char *s, size_t *width, char *buf, size_t bufsiz, const char *safechars); extern size_t mbs_safe_encode_size(size_t bytes); extern size_t mbs_safe_decode_size(const char *s); extern char *mbs_invalid_encode(const char *s, size_t *width); -extern char *mbs_invalid_encode_to_buffer(const char *s, size_t *width, char *buf); +extern char *mbs_invalid_encode_to_buffer(const char *s, size_t *width, char *buf, size_t bufsiz); #endif /* UTIL_LINUX_MBSALIGN_H */ diff --git a/lib/buffer.c b/lib/buffer.c index c453074021..bdc4744bc4 100644 --- a/lib/buffer.c +++ b/lib/buffer.c @@ -231,7 +231,7 @@ char *ul_buffer_get_safe_data(struct ul_buffer *buf, size_t *sz, size_t *width, buf->encoded_sz = encsz; } - res = mbs_safe_encode_to_buffer(data, &wsz, buf->encoded, safechars); + res = mbs_safe_encode_to_buffer(data, &wsz, buf->encoded, buf->encoded_sz, safechars); if (!res || !wsz || wsz == (size_t) -1) goto nothing; diff --git a/lib/mbsalign.c b/lib/mbsalign.c index b4ab7becdd..f94866b8b6 100644 --- a/lib/mbsalign.c +++ b/lib/mbsalign.c @@ -165,32 +165,39 @@ size_t mbs_safe_width(const char *s) * The @buf has to be big enough to store mbs_safe_encode_size(strlen(s))) * bytes. */ -char *mbs_safe_encode_to_buffer(const char *s, size_t *width, char *buf, const char *safechars) +char *mbs_safe_encode_to_buffer(const char *s, size_t *width, char *buf, size_t bufsiz, const char *safechars) { const char *p = s; char *r; + int rsz; size_t sz = s ? strlen(s) : 0; #ifdef HAVE_WIDECHAR mbstate_t st; memset(&st, 0, sizeof(st)); #endif - if (!sz || !buf) + if (!sz || !buf || !bufsiz) return NULL; r = buf; + rsz = (int) bufsiz; *width = 0; while (p && *p) { if (safechars && strchr(safechars, *p)) { + if (rsz < 2) + break; *r++ = *p++; + rsz--; continue; } if ((*p == '\\' && *(p + 1) == 'x') || iscntrl((unsigned char) *p)) { - sprintf(r, "\\x%02x", (unsigned char) *p); + if (snprintf(r, rsz, "\\x%02x", (unsigned char) *p) < 4) + break; r += 4; + rsz -= 4; *width += 4; p++; } @@ -209,35 +216,52 @@ char *mbs_safe_encode_to_buffer(const char *s, size_t *width, char *buf, const c * printable char according to the current locales. */ if (!isprint((unsigned char) *p)) { - sprintf(r, "\\x%02x", (unsigned char) *p); + if (snprintf(r, rsz, "\\x%02x", (unsigned char) *p) < 4) + break; r += 4; + rsz -= 4; *width += 4; } else { + if (rsz < 2) + break; (*width)++; *r++ = *p; + rsz--; } } else if (!iswprint(wc)) { size_t i; for (i = 0; i < len; i++) { - sprintf(r, "\\x%02x", (unsigned char) p[i]); + if (snprintf(r, rsz, "\\x%02x", (unsigned char) p[i]) < 4) + break; r += 4; + rsz -= 4; *width += 4; } + if (i < len) + break; } else { + if (rsz < (int)len + 1) + break; memcpy(r, p, len); r += len; + rsz -= len; *width += wcwidth(wc); } p += len; } #else else if (!isprint((unsigned char) *p)) { - sprintf(r, "\\x%02x", (unsigned char) *p); + if (snprintf(r, rsz, "\\x%02x", (unsigned char) *p) < 4) + break; p++; r += 4; + rsz -= 4; *width += 4; } else { + if (rsz < 2) + break; *r++ = *p++; + rsz--; (*width)++; } #endif @@ -254,20 +278,22 @@ char *mbs_safe_encode_to_buffer(const char *s, size_t *width, char *buf, const c * The @buf has to be big enough to store mbs_safe_encode_size(strlen(s))) * bytes. */ -char *mbs_invalid_encode_to_buffer(const char *s, size_t *width, char *buf) +char *mbs_invalid_encode_to_buffer(const char *s, size_t *width, char *buf, size_t bufsiz) { const char *p = s; char *r; + int rsz; size_t sz = s ? strlen(s) : 0; #ifdef HAVE_WIDECHAR mbstate_t st; memset(&st, 0, sizeof(st)); #endif - if (!sz || !buf) + if (!sz || !buf || !bufsiz) return NULL; r = buf; + rsz = (int) bufsiz; *width = 0; while (p && *p) { @@ -288,19 +314,29 @@ char *mbs_invalid_encode_to_buffer(const char *s, size_t *width, char *buf) * printable char according to the current locales. */ if (!isprint((unsigned char) *p)) { - sprintf(r, "\\x%02x", (unsigned char) *p); + if (snprintf(r, rsz, "\\x%02x", (unsigned char) *p) < 4) + break; r += 4; + rsz -= 4; *width += 4; } else { + if (rsz < 2) + break; (*width)++; *r++ = *p; + rsz--; } } else if (*p == '\\' && *(p + 1) == 'x') { - sprintf(r, "\\x%02x", (unsigned char) *p); + if (snprintf(r, rsz, "\\x%02x", (unsigned char) *p) < 4) + break; r += 4; + rsz -= 4; *width += 4; } else { + if (rsz < (int)len + 1) + break; r = mempcpy(r, p, len); + rsz -= len; *width += wcwidth(wc); } p += len; @@ -343,13 +379,15 @@ size_t mbs_safe_decode_size(const char *p) char *mbs_safe_encode(const char *s, size_t *width) { size_t sz = s ? strlen(s) : 0; + size_t bufsz; char *buf, *ret = NULL; if (!sz) return NULL; - buf = malloc(mbs_safe_encode_size(sz)); + bufsz = mbs_safe_encode_size(sz); + buf = malloc(bufsz); if (buf) - ret = mbs_safe_encode_to_buffer(s, width, buf, NULL); + ret = mbs_safe_encode_to_buffer(s, width, buf, bufsz, NULL); if (!ret) free(buf); return ret; @@ -362,13 +400,15 @@ char *mbs_safe_encode(const char *s, size_t *width) char *mbs_invalid_encode(const char *s, size_t *width) { size_t sz = s ? strlen(s) : 0; + size_t bufsz; char *buf, *ret = NULL; if (!sz) return NULL; - buf = malloc(mbs_safe_encode_size(sz)); + bufsz = mbs_safe_encode_size(sz); + buf = malloc(bufsz); if (buf) - ret = mbs_invalid_encode_to_buffer(s, width, buf); + ret = mbs_invalid_encode_to_buffer(s, width, buf, bufsz); if (!ret) free(buf); return ret; diff --git a/libsmartcols/src/print.c b/libsmartcols/src/print.c index a2c736ccad..e51046dc9a 100644 --- a/libsmartcols/src/print.c +++ b/libsmartcols/src/print.c @@ -975,7 +975,7 @@ int __scols_print_title(struct libscols_table *tb) goto done; } - if (!mbs_safe_encode_to_buffer(tb->title.data, &len, buf, NULL) || + if (!mbs_safe_encode_to_buffer(tb->title.data, &len, buf, bufsz, NULL) || !len || len == (size_t) -1) { rc = -EINVAL; goto done; -- 2.47.3