From e06fe487ab631c73ef7d4557e7dd69b09f528864 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Tue, 14 Jan 2025 01:27:59 +0800 Subject: [PATCH 1/4] RichString_setLen() minor logic fix It's okay to compare "<= RICHSTRING_MAXLEN". RICHSTRING_MAXLEN does not include the terminating L'\0' character and the internal buffer of a RichString instance has (RICHSTRING_MAXLEN + 1) elements. Signed-off-by: Kang-Che Sung --- RichString.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RichString.c b/RichString.c index 390beb073..afdb49b48 100644 --- a/RichString.c +++ b/RichString.c @@ -50,7 +50,7 @@ static void RichString_extendLen(RichString* this, int len) { } static void RichString_setLen(RichString* this, int len) { - if (len < RICHSTRING_MAXLEN && this->chlen < RICHSTRING_MAXLEN) { + if (len <= RICHSTRING_MAXLEN && this->chlen <= RICHSTRING_MAXLEN) { RichString_setChar(this, len, 0); this->chlen = len; } else { From f5d9fe859d97ff9243f1ffa50d8e97d9137fc6c8 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Tue, 14 Jan 2025 01:49:37 +0800 Subject: [PATCH 2/4] Add size limit check in RichString_extendLen() Prevent arithmetic overflows when computing sizes in RichString_extendLen(). Note that we are not using xMallocArray() and xReallocArray() as they would add unnecessary overflow checks in this particular case. Signed-off-by: Kang-Che Sung --- RichString.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/RichString.c b/RichString.c index afdb49b48..5e63038f5 100644 --- a/RichString.c +++ b/RichString.c @@ -22,11 +22,17 @@ in the source distribution for its full text. #define charBytes(n) (sizeof(CharType) * (n)) static void RichString_extendLen(RichString* this, int len) { + // TODO: Remove the "len" type casts once all the length properties + // of RichString have been upgraded to size_t. + if ((size_t)len > (SIZE_MAX - 1) / sizeof(CharType)) { + fail(); + } + if (this->chptr == this->chstr) { // String is in internal buffer if (len > RICHSTRING_MAXLEN) { // Copy from internal buffer to allocated string - this->chptr = xMalloc(charBytes(len + 1)); + this->chptr = xMalloc(charBytes((size_t)len + 1)); memcpy(this->chptr, this->chstr, charBytes(this->chlen)); } else { // Still fits in internal buffer, do nothing @@ -36,7 +42,7 @@ static void RichString_extendLen(RichString* this, int len) { // String is managed externally if (len > RICHSTRING_MAXLEN) { // Just reallocate the buffer accordingly - this->chptr = xRealloc(this->chptr, charBytes(len + 1)); + this->chptr = xRealloc(this->chptr, charBytes((size_t)len + 1)); } else { // Move string into internal buffer and free resources memcpy(this->chstr, this->chptr, charBytes(len)); From 64e91426878c4ba78c915ae169ae7ab5aa5c601b Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Tue, 14 Jan 2025 02:00:37 +0800 Subject: [PATCH 3/4] Prevent length wraparound in RichString_rewind() The computed length could wrap to a negative integer, causing undefined behavior. (Currently it would cause out-of-bound array access due to a signed length property in RichString. The length property should be migrated to an unsigned size_t type, but that would be done in a later commit.) Signed-off-by: Kang-Che Sung --- RichString.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RichString.c b/RichString.c index 5e63038f5..159d08ca0 100644 --- a/RichString.c +++ b/RichString.c @@ -65,7 +65,7 @@ static void RichString_setLen(RichString* this, int len) { } void RichString_rewind(RichString* this, int count) { - RichString_setLen(this, this->chlen - count); + RichString_setLen(this, this->chlen > count ? this->chlen - count : 0); } #ifdef HAVE_LIBNCURSESW From ffcc552606002ac1440b8432937b64423aef2db0 Mon Sep 17 00:00:00 2001 From: Explorer09 Date: Tue, 14 Jan 2025 02:11:39 +0800 Subject: [PATCH 4/4] Fix mbstowcs_nonfatal() that might convert string shorter than desired The "n" parameter of mbstowcs_nonfatal() refers to number of wide characters. But the logic seemed to be confused with "n" parameter of mbrtowc() (the number of bytes). Signed-off-by: Kang-Che Sung --- RichString.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/RichString.c b/RichString.c index 159d08ca0..7d0f82aef 100644 --- a/RichString.c +++ b/RichString.c @@ -12,6 +12,7 @@ in the source distribution for its full text. #include #include #include // IWYU pragma: keep +#include #include #include @@ -75,8 +76,8 @@ static size_t mbstowcs_nonfatal(wchar_t* restrict dest, const char* restrict src mbstate_t ps = { 0 }; bool broken = false; - while (n > 0) { - size_t ret = mbrtowc(dest, src, n, &ps); + while (written < n) { + size_t ret = mbrtowc(dest, src, SIZE_MAX, &ps); if (ret == (size_t)-1 || ret == (size_t)-2) { if (!broken) { broken = true; @@ -84,7 +85,6 @@ static size_t mbstowcs_nonfatal(wchar_t* restrict dest, const char* restrict src written++; } src++; - n--; continue; } @@ -97,7 +97,6 @@ static size_t mbstowcs_nonfatal(wchar_t* restrict dest, const char* restrict src dest++; written++; src += ret; - n -= ret; } return written;