aboutsummaryrefslogtreecommitdiff
path: root/tests/unit
diff options
context:
space:
mode:
authorNiall <Niall.oReilly@ucd.ie>2019-11-14 19:21:09 +0000
committerDaniel Stenberg <daniel@haxx.se>2019-11-16 16:15:03 +0100
commitb6a53fff6c1d07e8a9cb65ca1066d99490fb8132 (patch)
tree8f8b8ee97d9de60d29c0235953b37f798523769b /tests/unit
parent7627a2dd9d4b7417672fdec3dc6e7f8d3de379de (diff)
doh: improced both encoding and decoding
Improved estimation of expected_len and updated related comments; increased strictness of QNAME-encoding, adding error detection for empty labels and names longer than the overall limit; avoided treating DNAME as unexpected; updated unit test 1655 with more thorough set of proofs and tests Closes #4598
Diffstat (limited to 'tests/unit')
-rw-r--r--tests/unit/unit1655.c118
1 files changed, 91 insertions, 27 deletions
diff --git a/tests/unit/unit1655.c b/tests/unit/unit1655.c
index 7fea134d5..cccaab8da 100644
--- a/tests/unit/unit1655.c
+++ b/tests/unit/unit1655.c
@@ -36,43 +36,99 @@ static void unit_stop(void)
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
+/*
+ * Prove detection of write overflow using a short buffer and a name
+ * of maximal valid length.
+ *
+ * Prove detection of other invalid input.
*/
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";
+ const char *max =
+ /* ..|....1.........2.........3.........4.........5.........6... */
+ /* 3456789012345678901234567890123456789012345678901234567890123 */
+ "this.is.a.maximum-length.hostname." /* 34: 34 */
+ "with-no-label-of-greater-length-than-the-sixty-three-characters."
+ /* 64: 98 */
+ "specified.in.the.RFCs." /* 22: 120 */
+ "and.with.a.QNAME.encoding.whose.length.is.exactly." /* 50: 170 */
+ "the.maximum.length.allowed." /* 27: 197 */
+ "that.is.two-hundred.and.fifty-six." /* 34: 231 */
+ "including.the.last.null." /* 24: 255 */
+ "";
+ const char *toolong =
+ /* ..|....1.........2.........3.........4.........5.........6... */
+ /* 3456789012345678901234567890123456789012345678901234567890123 */
+ "here.is.a.hostname.which.is.just.barely.too.long." /* 49: 49 */
+ "to.be.encoded.as.a.QNAME.of.the.maximum.allowed.length."
+ /* 55: 104 */
+ "which.is.256.including.a.final.zero-length.label." /* 49: 153 */
+ "representing.the.root.node.so.that.a.name.with." /* 47: 200 */
+ "a.trailing.dot.may.have.up.to." /* 30: 230 */
+ "255.characters.never.more." /* 26: 256 */
+ "";
+ const char *emptylabel =
+ "this.is.an.otherwise-valid.hostname."
+ ".with.an.empty.label.";
+ const char *outsizelabel =
+ "this.is.an.otherwise-valid.hostname."
+ "with-a-label-of-greater-length-than-the-sixty-three-characters-"
+ "specified.in.the.RFCs.";
+
+ struct test {
+ const char *name;
+ const DOHcode expected_result;
+ };
/* plays the role of struct dnsprobe in urldata.h */
struct demo {
- unsigned char dohbuffer[512];
+ unsigned char dohbuffer[255 + 16]; /* deliberately short buffer */
unsigned char canary1;
unsigned char canary2;
unsigned char canary3;
};
- size_t olen = 100000;
- struct demo victim;
- DOHcode d;
- victim.canary1 = 87; /* magic numbers, arbritrarily picked */
- victim.canary2 = 35;
- victim.canary3 = 41;
- 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");
+ const struct test playlist[4] = {
+ { toolong, DOH_DNS_NAME_TOO_LONG }, /* expect early failure */
+ { emptylabel, DOH_DNS_BAD_LABEL }, /* also */
+ { outsizelabel, DOH_DNS_BAD_LABEL }, /* also */
+ { max, DOH_OK } /* expect buffer overwrite */
+ };
+
+ for(int i = 0; i < (int)(sizeof(playlist)/sizeof(*playlist)); i++) {
+ const char *name = playlist[i].name;
+ size_t olen = 100000;
+ struct demo victim;
+ DOHcode d;
+
+ victim.canary1 = 87; /* magic numbers, arbritrarily picked */
+ victim.canary2 = 35;
+ victim.canary3 = 41;
+ d = doh_encode(name, DNS_TYPE_A, victim.dohbuffer,
+ sizeof(struct demo), /* allow room for overflow */
+ &olen);
+
+ fail_unless(d == playlist[i].expected_result,
+ "result returned was not as expected");
+ if(d == playlist[i].expected_result) {
+ if(name == max) {
+ fail_if(victim.canary1 == 87,
+ "demo one-byte buffer overwrite did not happen");
+ }
+ else {
+ 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");
+ }
+ else {
+ if(d == DOH_OK) {
+ fail_unless(olen <= sizeof(victim.dohbuffer), "wrote outside bounds");
+ fail_unless(olen > strlen(name), "unrealistic low size");
+ }
+ }
}
} while(0);
@@ -84,6 +140,7 @@ do {
const size_t magic1 = 9765;
size_t olen1 = magic1;
const char *sunshine1 = "a.com";
+ const char *dotshine1 = "a.com.";
const char *sunshine2 = "aa.com";
size_t olen2;
DOHcode ret2;
@@ -94,6 +151,13 @@ do {
fail_if(olen1 == magic1, "olen has not been assigned properly");
fail_unless(olen1 > strlen(sunshine1), "bad out length");
+ /* with a trailing dot, the response should have the same length */
+ olen2 = magic1;
+ ret2 = doh_encode(dotshine1, dnstype, buffer, buflen, &olen2);
+ fail_unless(ret2 == DOH_OK, "dotshine case should pass fine");
+ fail_if(olen2 == magic1, "olen has not been assigned properly");
+ fail_unless(olen1 == olen2, "olen should not grow for a trailing dot");
+
/* add one letter, the response should be one longer */
olen2 = magic1;
ret2 = doh_encode(sunshine2, dnstype, buffer, buflen, &olen2);