From bbfc62ea8c1bedcfec3cad98dd8e3203c70b269e Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Sun, 2 Nov 2003 17:43:27 +0000 Subject: [PATCH] Fixed bug in overlap check in strncpy() -- it was assuming the src was 'n' bytes longs, when it could be shorter, which could cause false positives. Added an example of this to the regtest. MERGE TO STABLE git-svn-id: svn://svn.valgrind.org/valgrind/trunk@1983 --- memcheck/mac_replace_strmem.c | 10 ++++++---- memcheck/tests/overlap.c | 9 +++++++++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/memcheck/mac_replace_strmem.c b/memcheck/mac_replace_strmem.c index 4e4ec3d7a..1a9255eb4 100644 --- a/memcheck/mac_replace_strmem.c +++ b/memcheck/mac_replace_strmem.c @@ -186,13 +186,15 @@ char* strcpy ( char* dst, const char* src ) char* strncpy ( char* dst, const char* src, int n ) { - Char* dst_orig = dst; + const Char* src_orig = src; + Char* dst_orig = dst; Int m = 0; - if (is_overlap(dst, src, n, n)) - complain3("strncpy", dst, src, n); - while (m < n && *src) { m++; *dst++ = *src++; } + /* Check for overlap after copying; all n bytes of dst are relevant, + but only m+1 bytes of src if terminator was found */ + if (is_overlap(dst_orig, src_orig, n, (m < n) ? m+1 : n)) + complain3("strncpy", dst, src, n); while (m++ < n) *dst++ = 0; /* must pad remainder with nulls */ return dst_orig; diff --git a/memcheck/tests/overlap.c b/memcheck/tests/overlap.c index 60e14c20f..d868886f3 100644 --- a/memcheck/tests/overlap.c +++ b/memcheck/tests/overlap.c @@ -112,5 +112,14 @@ int main(void) strncat(a+20, a, 21); // run twice to check 2nd error isn't shown strncat(a, a+20, 21); + /* This is ok, but once gave a warning when strncpy() was wrong, + and used 'n' for the length, even when the src was shorter than 'n' */ + { + char dest[64]; + char src [16]; + strcpy( src, "short" ); + strncpy( dest, src, 20 ); + } + return 0; }