[Toybox] [PATCH] patch: fix out of bounds read on error message.

Eric Roshan-Eisner edre at google.com
Tue Jan 10 21:56:43 PST 2023


ASAN previously would crash on this new test. The filename passed to
error_exit() was not zeroed, and would allow printf to scan into the heap
looking for the null terminator.
---
 tests/patch.test   | 4 +++-
 toys/posix/patch.c | 6 +++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/patch.test b/tests/patch.test
index c18bfe1e..9f44e07c 100755
--- a/tests/patch.test
+++ b/tests/patch.test
@@ -90,11 +90,13 @@ testing "filter timestamps" "patch > /dev/null && cat bork" \
 testing "quoted name" "patch > /dev/null && cat 'fruit bat'" \
   "hello\n" "" '
 --- /dev/null
-+++ "fruit bat"
++++ "fruit ba\164"
 @@ -0,0 +1 @@
 +hello
 '
 
+testing "bad quote" "patch 2>&1" $'patch: bad "filename\n' "" '--- "filename'
+
 testing "dry run doesn't delete file" \
   "patch --dry-run > /dev/null && [ -e 'fruit bat' ] && echo yes" "yes\n" "" '
 --- "fruit bat"
diff --git a/toys/posix/patch.c b/toys/posix/patch.c
index eebcddaa..34a75e23 100644
--- a/toys/posix/patch.c
+++ b/toys/posix/patch.c
@@ -279,13 +279,13 @@ done:
 // read a filename that has been quoted or escaped
 static char *unquote_file(char *filename)
 {
-  char *s = filename, *t;
+  char *s = filename, *t, *newfile;
 
   // Return copy of file that wasn't quoted
   if (*s++ != '"' || !*s) return xstrdup(filename);
 
   // quoted and escaped filenames are larger than the original
-  for (t = filename = xmalloc(strlen(s) + 1); *s != '"'; s++) {
+  for (t = newfile = xmalloc(strlen(s) + 1); *s != '"'; s++) {
     if (!s[1]) error_exit("bad %s", filename);
 
     // don't accept escape sequences unless the filename is quoted
@@ -300,7 +300,7 @@ static char *unquote_file(char *filename)
   }
   *t = 0;
 
-  return filename;
+  return newfile;
 }
 
 // Read a patch file and find hunks, opening/creating/deleting files.
-- 
2.39.0.314.g84b9a713c41-goog



More information about the Toybox mailing list