Message ID | 20181010091215.7940-1-bshastry@sect.tu-berlin.de |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev] ossfuzz: Bug fix in odp and expr parse targets | expand |
On Wed, Oct 10, 2018 at 11:12:15AM +0200, bshastry@sect.tu-berlin.de wrote: > From: Bhargava Shastry <bshastry@sect.tu-berlin.de> > > This patch fixes a bug in the following test harnesses > - odp_target.c > - expr_parse_target.c > > The bug is as follows: > > We expect the fuzzed input to be a C string that does not contain a new > line character. This is because, the test code in OvS is built on > expecting string to not have a newline character (see for instance, > calls to ds_get_line() in test-odp.c etc.). > > The way we ensure fuzzed data is such a C string is as follows: > - Check size > 1 AND > - Check data[size - 1] is '\0' (NUL termination) AND > - Check that there is no '\n' in the C string that starts at data > > The third check is implemented using strchr. Our earlier logic was that, > were the C string to contain '\n', strchr would have a non-zero return > that can then be used to bail out early. > > The problem with this logic is that it does not consider the corner case > when data actually points to two or more C strings, like so: > \x01\x00\x0a\0x00 > > For this data sequence, strchr correctly returns "there is no newline > character" (in the first C string that is part of the sequence). > > But the data that is eventually passed to the fuzzed API > is the entire sequence of strings that may contain a new line in > between. > > This patch fixes the bug by adding an additional check: > - Check length of C string pointed to by data is actually equal to one > less than (due to NUL termination) size. > > This ensures that we are passing one and only one C string not > containing new line character to the fuzzed APIs. > > Signed-off-by: Bhargava Shastry <bshastry@sect.tu-berlin.de> Thanks, applied to master. I made minor style changes to make it better match how we usually write code.
diff --git a/tests/oss-fuzz/expr_parse_target.c b/tests/oss-fuzz/expr_parse_target.c index d72ad40c4..ca739012c 100644 --- a/tests/oss-fuzz/expr_parse_target.c +++ b/tests/oss-fuzz/expr_parse_target.c @@ -436,7 +436,8 @@ LLVMFuzzerTestOneInput(const uint8_t *input_, size_t size) { /* Bail out if we cannot construct at least a 1 char string. */ const char *input = (const char *) input_; - if (size < 2 || input[size - 1] != '\0' || strchr(input, '\n')) { + if (size < 2 || input[size - 1] != '\0' || strchr(input, '\n') || + (strlen(input) != size - 1)) { return 0; } diff --git a/tests/oss-fuzz/odp_target.c b/tests/oss-fuzz/odp_target.c index 93231bde3..b185174f6 100644 --- a/tests/oss-fuzz/odp_target.c +++ b/tests/oss-fuzz/odp_target.c @@ -125,7 +125,8 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { /* Bail out if we cannot construct at least a 1 char string. */ const char *input = (const char *) data; - if (size < 2 || input[size - 1] != '\0' || strchr(input, '\n')) { + if (size < 2 || input[size - 1] != '\0' || strchr(input, '\n') || + (strlen(input) != size - 1)) { return 0; }