[PATCH] common_prefix: be more careful about pathspec bounds
- From
Thomas Rast <trast@student.ethz.ch>
- Date
- Jun 10, 2010, 18:24 UTC
- Message-ID
- <825550ec93610c2d3c7dae7550729d96fc6cebbc.1276194169.git.trast@student.ethz.ch>
common_prefix() scans backwards from the far end of each 'next' pathspec, starting from 'len', shortening the 'prefix' using 'path' as a reference.
However, there was a small opportunity for an out-of-bounds access: len is unconditionally set to prefix-1 after a "direct match" test failed. This means that if 'next' is shorter than prefix+2, we read past it.
Normally this won't be a problem, which is probably why nobody has noticed that this was broken since 2006. Even if we find a slash somewhere beyond the actual contents of 'next', the memcmp after it can never match because of the terminating NUL. However, if we are unlucky and 'next' is right before a page boundary, we may not have any read access beyond it.
To fix, only set len to prefix-1 if that is actually inside 'next', i.e., reduces the available length. As explained in the last paragraph, increasing the length never results in more matches.
Signed-off-by: Thomas Rast <trast@student.ethz.ch> ---
Found by valgrind.
dir.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/dir.c b/dir.c index 5615f33..ca689ff 100644 --- a/dir.c +++ b/dir.c @@ -34,9 +34,11 @@ static int common_prefix(const char **pathspec) prefix = slash - path + 1; while ((next = *++pathspec) != NULL) { int len = strlen(next); - if (len >= prefix && !memcmp(path, next, prefix)) - continue; - len = prefix - 1; + if (len >= prefix) { + if (!memcmp(path, next, prefix)) + continue; + len = prefix - 1; + } for (;;) { if (!len) return 0;
-- 1.7.1.553.ga798e