From 433296f8802ccf82eb32da3bfa002df2a8b5095a Mon Sep 17 00:00:00 2001 From: Andy Chu Date: Sat, 19 Mar 2016 23:11:30 -0700 Subject: [PATCH] Fix a buffer overflow in diff -r. We were doing two 32-byte memset()s instead of two 16-byte memset()s. 'dir' referred to the instance (array of 2) and not the struct type. Add some test coverage for diff, including a case that hit this bug. The bug was found by running cp.test under AddressSanitizer, since it happens to use diff. --- tests/diff.test | 30 ++++++++++++++++++++++++++++++ toys/pending/diff.c | 6 +++--- 2 files changed, 33 insertions(+), 3 deletions(-) create mode 100755 tests/diff.test diff --git a/tests/diff.test b/tests/diff.test new file mode 100755 index 000000000..ca0b682ba --- /dev/null +++ b/tests/diff.test @@ -0,0 +1,30 @@ +#!/bin/bash + +#testing "name" "command" "result" "infile" "stdin" + +seq 10 > left +seq 11 > right + +expected='--- left ++++ right +@@ -8,3 +8,4 @@ + 8 + 9 + 10 ++11 +' +# Hm this only gives unified diffs? +testing "simple" "diff left right" "$expected" "" "" + + +expected='--- tree1/file ++++ tree2/file +@@ -1 +1 @@ +-foo ++food +' +mkdir -p tree1 tree2 +echo foo > tree1/file +echo food > tree2/file + +testing "simple" "diff -r tree1 tree2 |tee out" "$expected" "" "" diff --git a/toys/pending/diff.c b/toys/pending/diff.c index da6c13a02..53bdbce31 100644 --- a/toys/pending/diff.c +++ b/toys/pending/diff.c @@ -59,7 +59,7 @@ struct diff { long a, b, c, d, prev, suff; }; -static struct dir { +static struct dir_t { char **list; int nr_elm; } dir[2]; @@ -69,7 +69,7 @@ struct candidate { struct candidate *prev, *next; }; -static struct file { +static struct file_t { FILE *fp; int len; } file[2]; @@ -797,7 +797,7 @@ void diff_main(void) if (S_ISDIR(st[0].st_mode) && S_ISDIR(st[1].st_mode)) { for (j = 0; j < 2; j++) { - memset(&dir[j], 0, sizeof(dir)); + memset(&dir[j], 0, sizeof(struct dir_t)); dirtree_flagread(files[j], DIRTREE_SYMFOLLOW, list_dir); dir[j].nr_elm = TT.size; //size updated in list_dir qsort(&(dir[j].list[1]), (TT.size - 1), sizeof(char*), cmp);