From b5fc583c276586e173f0374f37644db2997410f1 Mon Sep 17 00:00:00 2001 From: Ed Maste Date: Mon, 15 May 2017 17:51:01 +0000 Subject: bsdgrep: don't allow negative -A / -B / -C Previously, when given a negative -A/-B/-C argument bsdgrep would overflow the respective context flag(s) and exhibited surprising behavior. Fix this by removing unsignedness of Aflag/Bflag and erroring out if we're given a value < 0. Also adjust the type used to track 'tail' context in procfile() so that it accurately reflects the Aflag value rather than overflowing and losing trailing context. This also fixes an inconsistency previously existing between -n and -C "n" behavior. They are now both limited to LLONG_MAX, to be consistent. Add some test cases to make sure grep errors out properly for both negative context values as well as non-numeric context values rather than giving bogus matches. Submitted by: Kyle Evans Reviewed by: cem Differential Revision: https://reviews.freebsd.org/D10675 --- contrib/netbsd-tests/usr.bin/grep/t_grep.sh | 23 +++++++++++++++++++++++ usr.bin/grep/grep.c | 18 +++++++++++------- usr.bin/grep/grep.h | 2 +- usr.bin/grep/queue.c | 2 +- usr.bin/grep/util.c | 3 ++- 5 files changed, 38 insertions(+), 10 deletions(-) diff --git a/contrib/netbsd-tests/usr.bin/grep/t_grep.sh b/contrib/netbsd-tests/usr.bin/grep/t_grep.sh index ff9205a0d703..95e92b2d9d00 100755 --- a/contrib/netbsd-tests/usr.bin/grep/t_grep.sh +++ b/contrib/netbsd-tests/usr.bin/grep/t_grep.sh @@ -517,6 +517,28 @@ grep_nomatch_flags_body() atf_check -o empty grep -q -A 1 -e "B" test1 atf_check -o empty grep -q -C 1 -e "B" test1 } + +atf_test_case badcontext +badcontext_head() +{ + atf_set "descr" "Check for handling of invalid context arguments" +} +badcontext_body() +{ + printf "A\nB\nC\n" > test1 + + atf_check -s not-exit:0 -e ignore grep -A "-1" "B" test1 + + atf_check -s not-exit:0 -e ignore grep -B "-1" "B" test1 + + atf_check -s not-exit:0 -e ignore grep -C "-1" "B" test1 + + atf_check -s not-exit:0 -e ignore grep -A "B" "B" test1 + + atf_check -s not-exit:0 -e ignore grep -B "B" "B" test1 + + atf_check -s not-exit:0 -e ignore grep -C "B" "B" test1 +} # End FreeBSD atf_init_test_cases() @@ -551,5 +573,6 @@ atf_init_test_cases() atf_add_test_case egrep_sanity atf_add_test_case grep_sanity atf_add_test_case grep_nomatch_flags + atf_add_test_case badcontext # End FreeBSD } diff --git a/usr.bin/grep/grep.c b/usr.bin/grep/grep.c index 3af9090d32c6..56f9c58768fe 100644 --- a/usr.bin/grep/grep.c +++ b/usr.bin/grep/grep.c @@ -108,8 +108,8 @@ struct epat *dpattern, *fpattern; char re_error[RE_ERROR_BUF + 1]; /* Command-line flags */ -unsigned long long Aflag; /* -A x: print x lines trailing each match */ -unsigned long long Bflag; /* -B x: print x lines leading each match */ +long long Aflag; /* -A x: print x lines trailing each match */ +long long Bflag; /* -B x: print x lines leading each match */ bool Hflag; /* -H: always print file name */ bool Lflag; /* -L: only show names of files with no matches */ bool bflag; /* -b: show block numbers for each match */ @@ -351,7 +351,7 @@ main(int argc, char *argv[]) char **aargv, **eargv, *eopts; char *ep; const char *pn; - unsigned long long l; + long long l; unsigned int aargc, eargc, i; int c, lastc, needpattern, newarg, prevoptind; @@ -438,10 +438,11 @@ main(int argc, char *argv[]) case '5': case '6': case '7': case '8': case '9': if (newarg || !isdigit(lastc)) Aflag = 0; - else if (Aflag > LLONG_MAX / 10) { + else if (Aflag > LLONG_MAX / 10 - 1) { errno = ERANGE; err(2, NULL); } + Aflag = Bflag = (Aflag * 10) + (c - '0'); break; case 'C': @@ -454,14 +455,17 @@ main(int argc, char *argv[]) /* FALLTHROUGH */ case 'B': errno = 0; - l = strtoull(optarg, &ep, 10); - if (((errno == ERANGE) && (l == ULLONG_MAX)) || - ((errno == EINVAL) && (l == 0))) + l = strtoll(optarg, &ep, 10); + if (errno == ERANGE || errno == EINVAL) err(2, NULL); else if (ep[0] != '\0') { errno = EINVAL; err(2, NULL); + } else if (l < 0) { + errno = EINVAL; + err(2, "context argument must be non-negative"); } + if (c == 'A') Aflag = l; else if (c == 'B') diff --git a/usr.bin/grep/grep.h b/usr.bin/grep/grep.h index 56ce456abb64..906318c67000 100644 --- a/usr.bin/grep/grep.h +++ b/usr.bin/grep/grep.h @@ -115,7 +115,7 @@ extern bool Eflag, Fflag, Gflag, Hflag, Lflag, bflag, cflag, hflag, iflag, lflag, mflag, nflag, oflag, qflag, sflag, vflag, wflag, xflag; extern bool dexclude, dinclude, fexclude, finclude, lbflag, nullflag; -extern unsigned long long Aflag, Bflag; +extern long long Aflag, Bflag; extern long long mcount; extern long long mlimit; extern char fileeol; diff --git a/usr.bin/grep/queue.c b/usr.bin/grep/queue.c index 453183367b8c..272dedfc91dc 100644 --- a/usr.bin/grep/queue.c +++ b/usr.bin/grep/queue.c @@ -49,7 +49,7 @@ struct qentry { }; static STAILQ_HEAD(, qentry) queue = STAILQ_HEAD_INITIALIZER(queue); -static unsigned long long count; +static long long count; static struct qentry *dequeue(void); diff --git a/usr.bin/grep/util.c b/usr.bin/grep/util.c index cfcc72d8cbd6..e4d99c727779 100644 --- a/usr.bin/grep/util.c +++ b/usr.bin/grep/util.c @@ -196,11 +196,12 @@ int procfile(const char *fn) { struct parsec pc; + long long tail; struct file *f; struct stat sb; struct str *ln; mode_t s; - int c, last_outed, t, tail; + int c, last_outed, t; bool doctx, printmatch, same_file; if (strcmp(fn, "-") == 0) { -- cgit v1.2.3