fortify-headers

standalone fortify-source implementation
git clone git://git.2f30.org/fortify-headers
Log | Files | Refs | README | LICENSE

commit e2cfd2879a15db00dfa9a42eeb1baaef6a930aff
parent c3b48c6b0bf501802295c85b1cf54275d6b74883
Author: jvoisin <julien.voisin@dustri.org>
Date:   Thu, 10 Oct 2024 15:50:40 +0200

Fix a crash in strncpy/stpncpy

```
Core was generated by `scripts/mod/modpost -M -m -o Module.symvers -n -T modules.order vmlinux.o'.
Program terminated with signal SIGSEGV, Segmentation fault.
warning: 17     src/string/strlen.c: No such file or directory
(gdb) bt
```

> I think strncpy logic is broken: `__fh_size_t max_len_s = strlen(__s);` may try read past `size_t __n`.
> Create a buf without any trailing `\0`, do `strncpy(dest, buf, sizeof(buf));`, it should work, since `strncpy` will stop at `sizeof buf`
> but the current fority-headers implementation will do `strlen(buf)`, which will go boom when it is not terminated with \0

Reported-by: ncopa

Diffstat:
Minclude/string.h | 12------------
Mtests/test_stpncpy_dynamic_write.c | 2+-
Mtests/test_stpncpy_overwrite_over.c | 2++
Mtests/test_stpncpy_overwrite_under.c | 2++
Mtests/test_strncpy_dynamic_write.c | 4+++-
Mtests/test_strncpy_overwrite_over.c | 2++
Mtests/test_strncpy_overwrite_under.c | 2++
Mtests/test_strncpy_static_write.c | 6++++--
8 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/string.h b/include/string.h @@ -208,12 +208,6 @@ _FORTIFY_FN(stpncpy) char *stpncpy(char * _FORTIFY_POS0 __d, const char *__s, #if __has_builtin(__builtin___stpncpy_chk) && FORTIFY_USE_NATIVE_CHK return __builtin___stpncpy_chk(__d, __s, __n, __fh_bos(__d, 0)); #else - __fh_size_t max_len_s = strlen(__s); - if (max_len_s > __n) - max_len_s = __n; - if (__fh_overlap(__d, max_len_s, __s, max_len_s)) - __builtin_trap(); - // If the length strlen(src) is smaller than n, the remaining // characters in the array pointed to by dest are filled with null // bytes ('\0') @@ -318,12 +312,6 @@ _FORTIFY_FN(strncpy) char *strncpy(char * _FORTIFY_POS0 __d, #if __has_builtin(__builtin___strncpy_chk) && FORTIFY_USE_NATIVE_CHK return __builtin___strncpy_chk(__d, __s, __n, __fh_bos(__d, 0)); #else - __fh_size_t max_len_s = strlen(__s); - if (max_len_s > __n) - max_len_s = __n; - if (__fh_overlap(__d, max_len_s, __s, max_len_s)) - __builtin_trap(); - // If the length of src is less than n, strncpy() writes additional // null bytes to dest to ensure that a total of n bytes are written. __fh_size_t __b = __fh_bos(__d, 0); diff --git a/tests/test_stpncpy_dynamic_write.c b/tests/test_stpncpy_dynamic_write.c @@ -3,7 +3,7 @@ #include <string.h> int main(int argc, char** argv) { - char buffer[] = {'A', 'B', 'C', 'D', 'E', 'F', '\0'}; + char buffer[] = {'A', 'B', 'C', 'D', 'E', 'F'}; stpncpy(buffer, "1234567", 3); puts(buffer); diff --git a/tests/test_stpncpy_overwrite_over.c b/tests/test_stpncpy_overwrite_over.c @@ -9,9 +9,11 @@ int main(int argc, char** argv) { stpncpy(buffer, buffer+5, 2); puts(buffer); +#if 0 CHK_FAIL_START stpncpy(buffer+1, buffer, 5); CHK_FAIL_END +#endif puts(buffer); return ret; diff --git a/tests/test_stpncpy_overwrite_under.c b/tests/test_stpncpy_overwrite_under.c @@ -10,9 +10,11 @@ int main(int argc, char** argv) { puts(buffer); char buffer2[] = {'A', 'A', 'A', 'A', 'B', 'B', 'B', 'B', '\0'}; +#if 0 CHK_FAIL_START stpncpy(buffer2-1, buffer2, 5); CHK_FAIL_END +#endif puts(buffer2); return ret; diff --git a/tests/test_strncpy_dynamic_write.c b/tests/test_strncpy_dynamic_write.c @@ -4,7 +4,9 @@ int main(int argc, char** argv) { char buffer[8] = {0}; - strncpy(buffer, "1234567", 5); + char src[] = {'A', 'B', 'C', 'D', 'E', 'F'}; + + strncpy(buffer, src, 5); puts(buffer); CHK_FAIL_START diff --git a/tests/test_strncpy_overwrite_over.c b/tests/test_strncpy_overwrite_over.c @@ -13,9 +13,11 @@ int main(int argc, char** argv) { puts(buffer); char buffer2[9] = {'A', 'A', 'A', 'A', 'B', 'B', 'B', 'B', '\0'}; +#if 0 CHK_FAIL_START strncpy(buffer2+1, buffer2, 5); CHK_FAIL_END +#endif puts(buffer2); return ret; diff --git a/tests/test_strncpy_overwrite_under.c b/tests/test_strncpy_overwrite_under.c @@ -6,9 +6,11 @@ int main(int argc, char** argv) { char buffer[9] = {'A', 'A', 'A', 'A', 'B', 'B', 'B', 'B', '\0'}; puts(buffer); +#if 0 CHK_FAIL_START strncpy(buffer-1, buffer, 5); CHK_FAIL_END +#endif puts(buffer); return ret; diff --git a/tests/test_strncpy_static_write.c b/tests/test_strncpy_static_write.c @@ -4,11 +4,13 @@ int main(int argc, char** argv) { char buffer[8] = {0}; - strncpy(buffer, "1234567", 5); + char src[] = {'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L'}; + + strncpy(buffer, src, 5); puts(buffer); CHK_FAIL_START - strncpy(buffer, "1234567890", 10); + strncpy(buffer, src, 10); CHK_FAIL_END puts(buffer);