aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRob Landley <rob@landley.net>2015-08-25 03:22:02 -0500
committerRob Landley <rob@landley.net>2015-08-25 03:22:02 -0500
commit9215cbc062f85cd285d8906a0b36941fa44d06c7 (patch)
tree81f7cf3ca91f1cc6a09dd36504bbbf737a8d023a
parent92f3b785690f5c23e1b84b6e726d7859d0ab1608 (diff)
downloadtoybox-9215cbc062f85cd285d8906a0b36941fa44d06c7.tar.gz
Static analysis from Hyejin Kim found possible pointer underflow.
Now that the kernel's 128k environment size has been lifted, it might be possible to feed in a gigabyte of suffix so argv[2] is enough larger than argv[1] that char *s decrements past NULL and points to arbitrary high memory (I.E. strlen(suffix) > (long)base), at which point the base > s test is defeated and we strcmp() against a wild pointer. Which is read only anyway and on 64 bit you probably couldn't hit any interesting addresses, but the fix is easy enough: compare strlen values instead of pointers. So do that instead.
-rw-r--r--toys/posix/basename.c6
1 files changed, 4 insertions, 2 deletions
diff --git a/toys/posix/basename.c b/toys/posix/basename.c
index c49a5f36..1a27a23b 100644
--- a/toys/posix/basename.c
+++ b/toys/posix/basename.c
@@ -24,8 +24,10 @@ void basename_main(void)
// chop off the suffix if provided
if (suffix) {
- char *s = base + strlen(base) - strlen(suffix);
- if (s > base && !strcmp(s, suffix)) *s = 0;
+ long bl = strlen(base), sl = strlen(suffix);
+ char *s = base + bl - sl;
+
+ if (bl > sl && !strcmp(s, suffix)) *s = 0;
}
puts(base);