commit c92c0cbc9f7a1ad889cfdda6943ca484d110438e Author: rl1987 <rl1987@sdf.lonestar.org> Date: Tue Dec 18 11:58:40 2018 +0200 Actually allow unrecognized address types in NETINFO cell Ignore the address value instead of failing with error condition in case unrecognized address type is found. --- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_netinfo.c | 48 +++++++++++++++++ src/trunnel/netinfo.c | 124 +++++++++++++++++++++++++------------------- src/trunnel/netinfo.trunnel | 4 +- 6 files changed, 125 insertions(+), 54 deletions(-) diff --git a/src/test/include.am b/src/test/include.am index dd2986c67..d90d78d58 100644 --- a/src/test/include.am +++ b/src/test/include.am @@ -147,6 +147,7 @@ src_test_test_SOURCES += \ src/test/test_logging.c \ src/test/test_mainloop.c \ src/test/test_microdesc.c \ + src/test/test_netinfo.c \ src/test/test_nodelist.c \ src/test/test_oom.c \ src/test/test_oos.c \ diff --git a/src/test/test.c b/src/test/test.c index 17b736d30..7f4f6017e 100644 --- a/src/test/test.c +++ b/src/test/test.c @@ -890,6 +890,7 @@ struct testgroup_t testgroups[] = { { "legacy_hs/", hs_tests }, { "link-handshake/", link_handshake_tests }, { "mainloop/", mainloop_tests }, + { "netinfo/", netinfo_tests }, { "nodelist/", nodelist_tests }, { "oom/", oom_tests }, { "oos/", oos_tests }, diff --git a/src/test/test.h b/src/test/test.h index 092356f0f..970a9b555 100644 --- a/src/test/test.h +++ b/src/test/test.h @@ -232,6 +232,7 @@ extern struct testcase_t link_handshake_tests[]; extern struct testcase_t logging_tests[]; extern struct testcase_t mainloop_tests[]; extern struct testcase_t microdesc_tests[]; +extern struct testcase_t netinfo_tests[]; extern struct testcase_t nodelist_tests[]; extern struct testcase_t oom_tests[]; extern struct testcase_t oos_tests[]; diff --git a/src/test/test_netinfo.c b/src/test/test_netinfo.c new file mode 100644 index 000000000..1fee2a1eb --- /dev/null +++ b/src/test/test_netinfo.c @@ -0,0 +1,48 @@ +/* Copyright (c) 2007-2018, The Tor Project, Inc. */ +/* See LICENSE for licensing information */ + +#include "orconfig.h" +#include "core/or/or.h" // XXX: is this needed? +#include "trunnel/netinfo.h" +#include "test/test.h" + +static void +test_netinfo_unsupported_addr(void *arg) +{ + const uint8_t wire_data[] = + { // TIME + 0x00, 0x00, 0x00, 0x01, + // OTHERADDR + 0x04, // ATYPE + 0x04, // ALEN + 0x08, 0x08, 0x08, 0x08, // AVAL + 0x01, // NMYADDR + 0x03, // ATYPE (unsupported) + 0x05, // ALEN + 'a', 'd', 'r', 'r', '!' // AVAL (unsupported) + }; + + (void)arg; + + netinfo_cell_t *parsed_cell = NULL; + + ssize_t parsed = netinfo_cell_parse(&parsed_cell, wire_data, + sizeof(wire_data)); + + tt_assert(parsed == sizeof(wire_data)); + + netinfo_addr_t *addr = netinfo_cell_get_my_addrs(parsed_cell, 0); + tt_assert(addr); + + tt_int_op(3, OP_EQ, netinfo_addr_get_addr_type(addr)); + tt_int_op(5, OP_EQ, netinfo_addr_get_len(addr)); + + done: + netinfo_cell_free(parsed_cell); +} + +struct testcase_t netinfo_tests[] = { + { "unsupported_addr", test_netinfo_unsupported_addr, 0, NULL, NULL }, + END_OF_TESTCASES +}; + diff --git a/src/trunnel/netinfo.c b/src/trunnel/netinfo.c index de389eb13..5d815b9b1 100644 --- a/src/trunnel/netinfo.c +++ b/src/trunnel/netinfo.c @@ -140,7 +140,6 @@ netinfo_addr_check(const netinfo_addr_t *obj) break; default: - return "Bad tag for union"; break; } return NULL; @@ -175,7 +174,6 @@ netinfo_addr_encoded_len(const netinfo_addr_t *obj) break; default: - trunnel_assert(0); break; } return result; @@ -198,6 +196,8 @@ netinfo_addr_encode(uint8_t *output, const size_t avail, const netinfo_addr_t *o const ssize_t encoded_len = netinfo_addr_encoded_len(obj); #endif + uint8_t *backptr_len = NULL; + if (NULL != (msg = netinfo_addr_check(obj))) goto check_failed; @@ -213,39 +213,49 @@ netinfo_addr_encode(uint8_t *output, const size_t avail, const netinfo_addr_t *o written += 1; ptr += 1; /* Encode u8 len */ + backptr_len = ptr; trunnel_assert(written <= avail); if (avail - written < 1) goto truncated; trunnel_set_uint8(ptr, (obj->len)); written += 1; ptr += 1; - - /* Encode union addr[addr_type] */ - trunnel_assert(written <= avail); - switch (obj->addr_type) { - - case NETINFO_ADDR_TYPE_IPV4: - - /* Encode u32 addr_ipv4 */ - trunnel_assert(written <= avail); - if (avail - written < 4) - goto truncated; - trunnel_set_uint32(ptr, trunnel_htonl(obj->addr_ipv4)); - written += 4; ptr += 4; - break; - - case NETINFO_ADDR_TYPE_IPV6: - - /* Encode u8 addr_ipv6[16] */ - trunnel_assert(written <= avail); - if (avail - written < 16) - goto truncated; - memcpy(ptr, obj->addr_ipv6, 16); - written += 16; ptr += 16; - break; - - default: - trunnel_assert(0); - break; + { + size_t written_before_union = written; + + /* Encode union addr[addr_type] */ + trunnel_assert(written <= avail); + switch (obj->addr_type) { + + case NETINFO_ADDR_TYPE_IPV4: + + /* Encode u32 addr_ipv4 */ + trunnel_assert(written <= avail); + if (avail - written < 4) + goto truncated; + trunnel_set_uint32(ptr, trunnel_htonl(obj->addr_ipv4)); + written += 4; ptr += 4; + break; + + case NETINFO_ADDR_TYPE_IPV6: + + /* Encode u8 addr_ipv6[16] */ + trunnel_assert(written <= avail); + if (avail - written < 16) + goto truncated; + memcpy(ptr, obj->addr_ipv6, 16); + written += 16; ptr += 16; + break; + + default: + break; + } + /* Write the length field back to len */ + trunnel_assert(written >= written_before_union); +#if UINT8_MAX < SIZE_MAX + if (written - written_before_union > UINT8_MAX) + goto check_failed; +#endif + trunnel_set_uint8(backptr_len, (written - written_before_union)); } @@ -291,29 +301,39 @@ netinfo_addr_parse_into(netinfo_addr_t *obj, const uint8_t *input, const size_t CHECK_REMAINING(1, truncated); obj->len = (trunnel_get_uint8(ptr)); remaining -= 1; ptr += 1; - - /* Parse union addr[addr_type] */ - switch (obj->addr_type) { - - case NETINFO_ADDR_TYPE_IPV4: - - /* Parse u32 addr_ipv4 */ - CHECK_REMAINING(4, truncated); - obj->addr_ipv4 = trunnel_ntohl(trunnel_get_uint32(ptr)); - remaining -= 4; ptr += 4; - break; - - case NETINFO_ADDR_TYPE_IPV6: - - /* Parse u8 addr_ipv6[16] */ - CHECK_REMAINING(16, truncated); - memcpy(obj->addr_ipv6, ptr, 16); - remaining -= 16; ptr += 16; - break; - - default: + { + size_t remaining_after; + CHECK_REMAINING(obj->len, truncated); + remaining_after = remaining - obj->len; + remaining = obj->len; + + /* Parse union addr[addr_type] */ + switch (obj->addr_type) { + + case NETINFO_ADDR_TYPE_IPV4: + + /* Parse u32 addr_ipv4 */ + CHECK_REMAINING(4, fail); + obj->addr_ipv4 = trunnel_ntohl(trunnel_get_uint32(ptr)); + remaining -= 4; ptr += 4; + break; + + case NETINFO_ADDR_TYPE_IPV6: + + /* Parse u8 addr_ipv6[16] */ + CHECK_REMAINING(16, fail); + memcpy(obj->addr_ipv6, ptr, 16); + remaining -= 16; ptr += 16; + break; + + default: + /* Skip to end of union */ + ptr += remaining; remaining = 0; + break; + } + if (remaining != 0) goto fail; - break; + remaining = remaining_after; } trunnel_assert(ptr + remaining == input + len_in); return len_in - remaining; diff --git a/src/trunnel/netinfo.trunnel b/src/trunnel/netinfo.trunnel index 83c3a9e40..2c4b7a759 100644 --- a/src/trunnel/netinfo.trunnel +++ b/src/trunnel/netinfo.trunnel @@ -7,10 +7,10 @@ const NETINFO_ADDR_TYPE_IPV6 = 6; struct netinfo_addr { u8 addr_type; u8 len; - union addr[addr_type] { + union addr[addr_type] with length len { NETINFO_ADDR_TYPE_IPV4: u32 ipv4; NETINFO_ADDR_TYPE_IPV6: u8 ipv6[16]; - default: fail; + default: ignore; }; }