commit ddd22b2f533db9c0da0bb262fbafa51f67c8587e
parent d6105aba5fd791e8d3f069e771517cdb947b5604
Author: jvoisin <julien.voisin@dustri.org>
Date: Fri, 1 May 2026 00:36:32 +0200
Fix strncat/wcsncat
Previously, no checks were done when __n <= __b, but strncat _appends_ after
existing content, making this a overly broad check check. For example, with an
8-byte buffer containing "12345\0", strncat(buf, "ABCD", 4) would have the
check skipped, but the result "12345ABCD\0" is 10 bytes, resulting in an
overflow.
This commit fixes this oversight, and adds a bunch of tests.
Diffstat:
17 files changed, 239 insertions(+), 29 deletions(-)
diff --git a/include/string.h b/include/string.h
@@ -140,14 +140,12 @@ _FORTIFY_FN(strncat) char *strncat(char * _FORTIFY_POS0 __d, const char *__s,
size_t __b = __bos(__d, 0);
size_t __sl, __dl;
- if (__n > __b) {
- __sl = strlen(__s);
- __dl = strlen(__d);
- if (__sl > __n)
- __sl = __n;
- if (__sl + __dl + 1 > __b)
- __builtin_trap();
- }
+ __sl = strlen(__s);
+ __dl = strlen(__d);
+ if (__sl > __n)
+ __sl = __n;
+ if (__sl + __dl + 1 > __b)
+ __builtin_trap();
return __orig_strncat(__d, __s, __n);
}
diff --git a/include/wchar.h b/include/wchar.h
@@ -153,14 +153,12 @@ _FORTIFY_FN(wcsncat) wchar_t *wcsncat(wchar_t * _FORTIFY_POS0 __d,
size_t __b = __bos(__d, 0);
size_t __sl, __dl;
- if (__n > __b / sizeof(wchar_t)) {
- __sl = wcslen(__s);
- __dl = wcslen(__d);
- if (__sl > __n)
- __sl = __n;
- if (__sl + __dl + 1 > __b / sizeof(wchar_t))
- __builtin_trap();
- }
+ __sl = wcslen(__s);
+ __dl = wcslen(__d);
+ if (__sl > __n)
+ __sl = __n;
+ if (__sl + __dl + 1 > __b / sizeof(wchar_t))
+ __builtin_trap();
return __orig_wcsncat(__d, __s, __n);
}
diff --git a/tests/Makefile b/tests/Makefile
@@ -84,6 +84,11 @@ RUNTIME_TARGETS= \
test_strlcpy_dynamic_write \
test_strlcpy_static_write \
test_strncat_dynamic_write \
+ test_strncat_n_eq_buf \
+ test_strncat_n_gt_buf \
+ test_strncat_n_lt_buf \
+ test_strncat_n_one \
+ test_strncat_safe \
test_strncat_static_write \
test_strncpy_dynamic_write \
test_strncpy_static_write \
@@ -101,6 +106,11 @@ RUNTIME_TARGETS= \
test_wcsnrtombs_static \
test_wcscat_static_write \
test_wcscpy_static_write \
+ test_wcsncat_n_eq_buf \
+ test_wcsncat_n_gt_buf \
+ test_wcsncat_n_lt_buf \
+ test_wcsncat_n_one \
+ test_wcsncat_safe \
test_wcsncat_static_write \
test_wcsncpy_static_write \
test_wmemcpy_dynamic_read \
diff --git a/tests/test_mbsnrtowcs_dynamic.c b/tests/test_mbsnrtowcs_dynamic.c
@@ -14,9 +14,7 @@ int main(int argc, char** argv) {
srcp = src;
mbsnrtowcs(buffer, &srcp, 2, 2, &st);
- /* Unsafe: ask to write argc (10) wide chars into 4-element buffer.
- * Before the fix, the else branch clamped source bytes instead of
- * the output wide-char count, allowing destination overflow. */
+ /* Unsafe: ask to write argc (10) wide chars into 4-element buffer. */
CHK_FAIL_START
srcp = src;
memset(&st, 0, sizeof(st));
diff --git a/tests/test_strncat_dynamic_write.c b/tests/test_strncat_dynamic_write.c
@@ -7,11 +7,9 @@ int main(int argc, char** argv) {
strncat(buffer, "1234567", 5);
puts(buffer);
-#if 0
CHK_FAIL_START
strncat(buffer, argv[1], argc);
CHK_FAIL_END
-#endif
puts(buffer);
return ret;
diff --git a/tests/test_strncat_n_eq_buf.c b/tests/test_strncat_n_eq_buf.c
@@ -0,0 +1,18 @@
+#include "common.h"
+
+#include <string.h>
+
+int main(int argc, char** argv) {
+ char buffer[8] = {0};
+ strncat(buffer, "12345", 5);
+ puts(buffer);
+
+ /* n == buffer size but overflow due to existing content.
+ * buffer has 5 chars, src "ABC" (len 3): 5+3+1 = 9 > 8 → overflow. */
+ CHK_FAIL_START
+ strncat(buffer, "ABC", 8);
+ CHK_FAIL_END
+
+ puts(buffer);
+ return ret;
+}
diff --git a/tests/test_strncat_n_gt_buf.c b/tests/test_strncat_n_gt_buf.c
@@ -0,0 +1,17 @@
+#include "common.h"
+
+#include <string.h>
+
+int main(int argc, char** argv) {
+ char buffer[8] = {0};
+ strncat(buffer, "12345", 5);
+ puts(buffer);
+
+ /* n > buffer size and overflow: n=10, buffer has 5 chars. */
+ CHK_FAIL_START
+ strncat(buffer, "1234567890", 10);
+ CHK_FAIL_END
+
+ puts(buffer);
+ return ret;
+}
diff --git a/tests/test_strncat_n_lt_buf.c b/tests/test_strncat_n_lt_buf.c
@@ -0,0 +1,18 @@
+#include "common.h"
+
+#include <string.h>
+
+int main(int argc, char** argv) {
+ char buffer[8] = {0};
+ strncat(buffer, "12345", 5);
+ puts(buffer);
+
+ /* n < buffer size but overflow due to existing content.
+ * buffer has 5 chars, src "ABCD" (len 4), min(4,3)=3: 5+3+1 = 9 > 8. */
+ CHK_FAIL_START
+ strncat(buffer, "ABCD", 3);
+ CHK_FAIL_END
+
+ puts(buffer);
+ return ret;
+}
diff --git a/tests/test_strncat_n_one.c b/tests/test_strncat_n_one.c
@@ -0,0 +1,18 @@
+#include "common.h"
+
+#include <string.h>
+
+int main(int argc, char** argv) {
+ char buffer[8] = {0};
+ strncat(buffer, "1234567", 7);
+ puts(buffer);
+
+ /* n=1, buffer has 7 chars ("1234567").
+ * 7+1+1 = 9 > 8 → overflow. Even n=1 can overflow a nearly-full buffer. */
+ CHK_FAIL_START
+ strncat(buffer, "X", 1);
+ CHK_FAIL_END
+
+ puts(buffer);
+ return ret;
+}
diff --git a/tests/test_strncat_safe.c b/tests/test_strncat_safe.c
@@ -0,0 +1,34 @@
+#include "common.h"
+
+#include <string.h>
+
+int main(int argc, char** argv) {
+ char buffer[8] = {0};
+
+ /* Safe: empty buffer, append 7 chars with n=7 → "1234567\0" = exactly 8 bytes */
+ strncat(buffer, "1234567", 7);
+ puts(buffer);
+
+ /* Safe: reset and append with n larger than source.
+ * src is "AB" (len 2), n=100 → only 2 chars copied + NUL = 3 bytes */
+ buffer[0] = '\0';
+ strncat(buffer, "AB", 100);
+ puts(buffer);
+
+ /* Safe: partially filled, append fits exactly.
+ * buffer = "ABCD" (4 chars), append "EFG" with n=3 → 4+3+1 = 8 = exact fit */
+ buffer[0] = '\0';
+ strncat(buffer, "ABCD", 4);
+ strncat(buffer, "EFG", 3);
+ puts(buffer);
+
+ /* Safe: n limits copy to fit.
+ * buffer = "ABCDE" (5 chars), src = "FGHIJKLM" (8 chars), n=2 → 5+2+1 = 8 */
+ buffer[0] = '\0';
+ strncat(buffer, "ABCDE", 5);
+ strncat(buffer, "FGHIJKLM", 2);
+ puts(buffer);
+
+ /* All safe operations passed without trapping */
+ return 0;
+}
diff --git a/tests/test_strncat_static_write.c b/tests/test_strncat_static_write.c
@@ -4,15 +4,15 @@
int main(int argc, char** argv) {
char buffer[8] = {0};
- char src[] = {'1', '2', '3', '4', '5', '6', '7', '8', '9', '0'};
- strncat(buffer, src, 5);
+ strncat(buffer, "12345", 5);
puts(buffer);
-#if 0
+ /* n=4 is less than buffer size (8), but buffer already has 5 chars,
+ * so appending 4 more + NUL = 10 bytes total, overflowing the buffer.
+ */
CHK_FAIL_START
- strncat(buffer, src, 10);
+ strncat(buffer, "ABCD", 4);
CHK_FAIL_END
-#endif
puts(buffer);
return ret;
diff --git a/tests/test_wcsncat_n_eq_buf.c b/tests/test_wcsncat_n_eq_buf.c
@@ -0,0 +1,18 @@
+#include "common.h"
+
+#include <wchar.h>
+
+int main(int argc, char** argv) {
+ wchar_t buffer[8] = {0};
+ wcsncat(buffer, L"12345", 5);
+ printf("%ls\n", buffer);
+
+ /* n == buffer capacity but overflow due to existing content.
+ * buffer has 5 wchars, src L"ABC" (len 3): 5+3+1 = 9 > 8 → overflow. */
+ CHK_FAIL_START
+ wcsncat(buffer, L"ABC", 8);
+ CHK_FAIL_END
+
+ printf("%ls\n", buffer);
+ return ret;
+}
diff --git a/tests/test_wcsncat_n_gt_buf.c b/tests/test_wcsncat_n_gt_buf.c
@@ -0,0 +1,17 @@
+#include "common.h"
+
+#include <wchar.h>
+
+int main(int argc, char** argv) {
+ wchar_t buffer[8] = {0};
+ wcsncat(buffer, L"12345", 5);
+ printf("%ls\n", buffer);
+
+ /* n > buffer capacity and overflow: n=10, buffer has 5 wchars. */
+ CHK_FAIL_START
+ wcsncat(buffer, L"1234567890", 10);
+ CHK_FAIL_END
+
+ printf("%ls\n", buffer);
+ return ret;
+}
diff --git a/tests/test_wcsncat_n_lt_buf.c b/tests/test_wcsncat_n_lt_buf.c
@@ -0,0 +1,18 @@
+#include "common.h"
+
+#include <wchar.h>
+
+int main(int argc, char** argv) {
+ wchar_t buffer[8] = {0};
+ wcsncat(buffer, L"12345", 5);
+ printf("%ls\n", buffer);
+
+ /* n < buffer capacity but overflow due to existing content.
+ * buffer has 5 wchars, src L"ABCD" (len 4), min(4,3)=3: 5+3+1 = 9 > 8. */
+ CHK_FAIL_START
+ wcsncat(buffer, L"ABCD", 3);
+ CHK_FAIL_END
+
+ printf("%ls\n", buffer);
+ return ret;
+}
diff --git a/tests/test_wcsncat_n_one.c b/tests/test_wcsncat_n_one.c
@@ -0,0 +1,18 @@
+#include "common.h"
+
+#include <wchar.h>
+
+int main(int argc, char** argv) {
+ wchar_t buffer[8] = {0};
+ wcsncat(buffer, L"1234567", 7);
+ printf("%ls\n", buffer);
+
+ /* n=1, buffer has 7 wchars.
+ * 7+1+1 = 9 > 8 → overflow. Even n=1 can overflow a nearly-full buffer. */
+ CHK_FAIL_START
+ wcsncat(buffer, L"X", 1);
+ CHK_FAIL_END
+
+ printf("%ls\n", buffer);
+ return ret;
+}
diff --git a/tests/test_wcsncat_safe.c b/tests/test_wcsncat_safe.c
@@ -0,0 +1,34 @@
+#include "common.h"
+
+#include <wchar.h>
+
+int main(int argc, char** argv) {
+ wchar_t buffer[8] = {0};
+
+ /* Safe: empty buffer, append 7 wide chars → exactly fills buffer */
+ wcsncat(buffer, L"ABCDEFG", 7);
+ printf("%ls\n", buffer);
+
+ /* Safe: reset, append with n larger than source.
+ * src is L"AB" (len 2), n=100 → only 2 wchars copied */
+ buffer[0] = L'\0';
+ wcsncat(buffer, L"AB", 100);
+ printf("%ls\n", buffer);
+
+ /* Safe: partially filled, append fits exactly.
+ * buffer = L"ABCD" (4 wchars), append L"EFG" n=3 → 4+3+1 = 8 = exact fit */
+ buffer[0] = L'\0';
+ wcsncat(buffer, L"ABCD", 4);
+ wcsncat(buffer, L"EFG", 3);
+ printf("%ls\n", buffer);
+
+ /* Safe: n limits copy to fit.
+ * buffer = L"ABCDE" (5 wchars), src long, n=2 → 5+2+1 = 8 = exact fit */
+ buffer[0] = L'\0';
+ wcsncat(buffer, L"ABCDE", 5);
+ wcsncat(buffer, L"FGHIJKLM", 2);
+ printf("%ls\n", buffer);
+
+ /* All safe operations passed without trapping */
+ return 0;
+}
diff --git a/tests/test_wcsnrtombs_dynamic.c b/tests/test_wcsnrtombs_dynamic.c
@@ -14,9 +14,7 @@ int main(int argc, char** argv) {
srcp = src;
wcsnrtombs(buffer, &srcp, 4, 4, &st);
- /* Unsafe: ask to write argc (10) bytes into 8-byte buffer.
- * Before the fix, the first branch incorrectly divided the byte-sized
- * buffer capacity by sizeof(wchar_t), making the check too permissive. */
+ /* Unsafe: ask to write argc (10) bytes into 8-byte buffer. */
CHK_FAIL_START
srcp = src;
memset(&st, 0, sizeof(st));
