diff options
author | Paul Dreik <github@pauldreik.se> | 2019-09-14 03:16:09 +0200 |
---|---|---|
committer | Daniel Stenberg <daniel@haxx.se> | 2019-09-15 23:25:24 +0200 |
commit | b7666027296a4f89a8ce6b22af335e8aee7a7782 (patch) | |
tree | e75c54442cdb8ed02be13d2a8d4dd7de16f90e2b /tests/unit | |
parent | 5eb75d418657a734bfbe02e5aef69cc5fe28ebc6 (diff) | |
download | curl-b7666027296a4f89a8ce6b22af335e8aee7a7782.tar.gz |
doh: fix (harmless) buffer overrun
Added unit test case 1655 to verify.
Close #4352
the code correctly finds the flaws in the old code,
if one temporarily restores doh.c to the old version.
Diffstat (limited to 'tests/unit')
-rw-r--r-- | tests/unit/CMakeLists.txt | 1 | ||||
-rw-r--r-- | tests/unit/Makefile.inc | 6 | ||||
-rw-r--r-- | tests/unit/README | 6 | ||||
-rw-r--r-- | tests/unit/unit1655.c | 110 |
4 files changed, 121 insertions, 2 deletions
diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt index 4b0cec4a8..bc0699231 100644 --- a/tests/unit/CMakeLists.txt +++ b/tests/unit/CMakeLists.txt @@ -22,6 +22,7 @@ set(UT_SRC # Broken link on Linux # unit1604.c unit1620.c + unit1655.c ) set(UT_COMMON_FILES ../libtest/first.c ../libtest/test.h curlcheck.h) diff --git a/tests/unit/Makefile.inc b/tests/unit/Makefile.inc index f3cba1c2a..45278ba81 100644 --- a/tests/unit/Makefile.inc +++ b/tests/unit/Makefile.inc @@ -11,7 +11,7 @@ UNITPROGS = unit1300 unit1301 unit1302 unit1303 unit1304 unit1305 unit1307 \ unit1399 \ unit1600 unit1601 unit1602 unit1603 unit1604 unit1605 unit1606 unit1607 \ unit1608 unit1609 unit1620 unit1621 \ - unit1650 unit1651 unit1652 unit1653 unit1654 + unit1650 unit1651 unit1652 unit1653 unit1654 unit1655 unit1300_SOURCES = unit1300.c $(UNITFILES) unit1300_CPPFLAGS = $(AM_CPPFLAGS) @@ -118,3 +118,7 @@ unit1653_CPPFLAGS = $(AM_CPPFLAGS) unit1654_SOURCES = unit1654.c $(UNITFILES) unit1654_CPPFLAGS = $(AM_CPPFLAGS) + +unit1655_SOURCES = unit1655.c $(UNITFILES) +unit1655_CPPFLAGS = $(AM_CPPFLAGS) + diff --git a/tests/unit/README b/tests/unit/README index b8a513b3b..060b670c6 100644 --- a/tests/unit/README +++ b/tests/unit/README @@ -35,6 +35,9 @@ We put tests that focus on an area or a specific function into a single C source file. The source file should be named 'unitNNNN.c' where NNNN is a number that starts with 1300 and you can pick the next free number. +Add your test to tests/unit/Makefile.inc (if it is a unit test). +Add your test data to tests/data/Makefile.inc + You also need a separate file called tests/data/testNNNN (using the same number) that describes your test case. See the test1300 file for inspiration and the tests/FILEFORMAT documentation. @@ -46,9 +49,10 @@ For the actual C file, here's a very simple example: #include "a libcurl header.h" /* from the lib dir */ -static void unit_setup( void ) +static CURLcode unit_setup( void ) { /* whatever you want done first */ + return CURLE_OK; } static void unit_stop( void ) diff --git a/tests/unit/unit1655.c b/tests/unit/unit1655.c new file mode 100644 index 000000000..60f43d7d6 --- /dev/null +++ b/tests/unit/unit1655.c @@ -0,0 +1,110 @@ +/*************************************************************************** + * _ _ ____ _ + * Project ___| | | | _ \| | + * / __| | | | |_) | | + * | (__| |_| | _ <| |___ + * \___|\___/|_| \_\_____| + * + * Copyright (C) 2019, Daniel Stenberg, <daniel@haxx.se>, et al. + * + * This software is licensed as described in the file COPYING, which + * you should have received as part of this distribution. The terms + * are also available at https://curl.haxx.se/docs/copyright.html. + * + * You may opt to use, copy, modify, merge, publish, distribute and/or sell + * copies of the Software, and permit persons to whom the Software is + * furnished to do so, under the terms of the COPYING file. + * + * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY + * KIND, either express or implied. + * + ***************************************************************************/ +#include "curlcheck.h" + +#include "doh.h" /* from the lib dir */ + +static CURLcode unit_setup(void) +{ + /* whatever you want done first */ + return CURLE_OK; +} + +static void unit_stop(void) +{ + /* done before shutting down and exiting */ +} + +UNITTEST_START + +/* introduce a scope and prove the corner case with write overflow, + * so we can prove this test would detect it and that it is properly fixed + */ +do { +const char *bad = "this.is.a.hostname.where.each.individual.part.is.within." + "the.sixtythree.character.limit.but.still.long.enough.to." + "trigger.the.the.buffer.overflow......it.is.chosen.to.be." + "of.a.length.such.that.it.causes.a.two.byte.buffer......." + "overwrite.....making.it.longer.causes.doh.encode.to....." + ".return.early.so.dont.change.its.length.xxxx.xxxxxxxxxxx" + "..xxxxxx.....xx..........xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" + "xxxxxxxxxxxxxxxxxxxxxxxxxx.xxxxxxxxxxxxxxxx..x......xxxx" + "xxxx..xxxxxxxxxxxxxxxxxxx.x...xxxx.x.x.x...xxxxx"; + +/* plays the role of struct dnsprobe in urldata.h */ +struct demo { + unsigned char dohbuffer[512]; + unsigned char canary1; + unsigned char canary2; + unsigned char canary3; +}; + +size_t olen = 100000; +struct demo victim; +victim.canary1 = 87; /* magic numbers, arbritrarily picked */ +victim.canary2 = 35; +victim.canary3 = 41; +DOHcode d = doh_encode(bad, DNS_TYPE_A, victim.dohbuffer, + sizeof(victim.dohbuffer), &olen); +fail_unless(victim.canary1 == 87, "one byte buffer overwrite has happened"); +fail_unless(victim.canary2 == 35, "two byte buffer overwrite has happened"); +fail_unless(victim.canary3 == 41, "three byte buffer overwrite has happened"); +if(d == DOH_OK) +{ + fail_unless(olen <= sizeof(victim.dohbuffer), "wrote outside bounds"); + fail_unless(olen > strlen(bad), "unrealistic low size"); +} +} while(0); + +/* run normal cases and try to trigger buffer length related errors */ +do { +DNStype dnstype = DNS_TYPE_A; +unsigned char buffer[128]; +const size_t buflen = sizeof(buffer); +const size_t magic1 = 9765; +size_t olen1 = magic1; +const char *sunshine1 = "a.com"; +const char *sunshine2 = "aa.com"; + +DOHcode ret = doh_encode(sunshine1, dnstype, buffer, buflen, &olen1); +fail_unless(ret == DOH_OK, "sunshine case 1 should pass fine"); +fail_if(olen1 == magic1, "olen has not been assigned properly"); +fail_unless(olen1 > strlen(sunshine1), "bad out length"); + +/* add one letter, the response should be one longer */ +size_t olen2 = magic1; +DOHcode ret2 = doh_encode(sunshine2, dnstype, buffer, buflen, &olen2); +fail_unless(ret2 == DOH_OK, "sunshine case 2 should pass fine"); +fail_if(olen2 == magic1, "olen has not been assigned properly"); +fail_unless(olen1 + 1 == olen2, "olen should grow with the hostname"); + +/* pass a short buffer, should fail */ +size_t olen; +ret = doh_encode(sunshine1, dnstype, buffer, olen1 - 1, &olen); +fail_if(ret == DOH_OK, "short buffer should have been noticed"); + +/* pass a minimum buffer, should succeed */ +ret = doh_encode(sunshine1, dnstype, buffer, olen1, &olen); +fail_unless(ret == DOH_OK, "minimal length buffer should be long enough"); +fail_unless(olen == olen1, "bad buffer length"); +} while(0); +UNITTEST_STOP |