diff --git a/common/test/Makefile b/common/test/Makefile index 75331d62e6dc..db0fbcaab70e 100644 --- a/common/test/Makefile +++ b/common/test/Makefile @@ -128,4 +128,6 @@ common/test/run-htable: \ common/test/run-shutdown_scriptpubkey: wire/towire.o wire/fromwire.o +common/test/run-wireaddr: wire/towire.o wire/fromwire.o + check-units: $(COMMON_TEST_PROGRAMS:%=unittest/%) diff --git a/common/test/run-wireaddr.c b/common/test/run-wireaddr.c index 6d3bd76f232c..faa48e3c7c46 100644 --- a/common/test/run-wireaddr.c +++ b/common/test/run-wireaddr.c @@ -58,69 +58,6 @@ u8 *b32_decode(const tal_t *ctx UNNEEDED, const char *str UNNEEDED, size_t len U /* Generated stub for b32_encode */ char *b32_encode(const tal_t *ctx UNNEEDED, const void *data UNNEEDED, size_t len UNNEEDED) { fprintf(stderr, "b32_encode called!\n"); abort(); } -/* Generated stub for fromwire */ -const u8 *fromwire(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, void *copy UNNEEDED, size_t n UNNEEDED) -{ fprintf(stderr, "fromwire called!\n"); abort(); } -/* Generated stub for fromwire_bool */ -bool fromwire_bool(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) -{ fprintf(stderr, "fromwire_bool called!\n"); abort(); } -/* Generated stub for fromwire_fail */ -void *fromwire_fail(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) -{ fprintf(stderr, "fromwire_fail called!\n"); abort(); } -/* Generated stub for fromwire_secp256k1_ecdsa_signature */ -void fromwire_secp256k1_ecdsa_signature(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, - secp256k1_ecdsa_signature *signature UNNEEDED) -{ fprintf(stderr, "fromwire_secp256k1_ecdsa_signature called!\n"); abort(); } -/* Generated stub for fromwire_sha256 */ -void fromwire_sha256(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, struct sha256 *sha256 UNNEEDED) -{ fprintf(stderr, "fromwire_sha256 called!\n"); abort(); } -/* Generated stub for fromwire_tal_arrn */ -u8 *fromwire_tal_arrn(const tal_t *ctx UNNEEDED, - const u8 **cursor UNNEEDED, size_t *max UNNEEDED, size_t num UNNEEDED) -{ fprintf(stderr, "fromwire_tal_arrn called!\n"); abort(); } -/* Generated stub for fromwire_u16 */ -u16 fromwire_u16(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) -{ fprintf(stderr, "fromwire_u16 called!\n"); abort(); } -/* Generated stub for fromwire_u32 */ -u32 fromwire_u32(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) -{ fprintf(stderr, "fromwire_u32 called!\n"); abort(); } -/* Generated stub for fromwire_u64 */ -u64 fromwire_u64(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) -{ fprintf(stderr, "fromwire_u64 called!\n"); abort(); } -/* Generated stub for fromwire_u8 */ -u8 fromwire_u8(const u8 **cursor UNNEEDED, size_t *max UNNEEDED) -{ fprintf(stderr, "fromwire_u8 called!\n"); abort(); } -/* Generated stub for fromwire_u8_array */ -void fromwire_u8_array(const u8 **cursor UNNEEDED, size_t *max UNNEEDED, u8 *arr UNNEEDED, size_t num UNNEEDED) -{ fprintf(stderr, "fromwire_u8_array called!\n"); abort(); } -/* Generated stub for towire */ -void towire(u8 **pptr UNNEEDED, const void *data UNNEEDED, size_t len UNNEEDED) -{ fprintf(stderr, "towire called!\n"); abort(); } -/* Generated stub for towire_bool */ -void towire_bool(u8 **pptr UNNEEDED, bool v UNNEEDED) -{ fprintf(stderr, "towire_bool called!\n"); abort(); } -/* Generated stub for towire_secp256k1_ecdsa_signature */ -void towire_secp256k1_ecdsa_signature(u8 **pptr UNNEEDED, - const secp256k1_ecdsa_signature *signature UNNEEDED) -{ fprintf(stderr, "towire_secp256k1_ecdsa_signature called!\n"); abort(); } -/* Generated stub for towire_sha256 */ -void towire_sha256(u8 **pptr UNNEEDED, const struct sha256 *sha256 UNNEEDED) -{ fprintf(stderr, "towire_sha256 called!\n"); abort(); } -/* Generated stub for towire_u16 */ -void towire_u16(u8 **pptr UNNEEDED, u16 v UNNEEDED) -{ fprintf(stderr, "towire_u16 called!\n"); abort(); } -/* Generated stub for towire_u32 */ -void towire_u32(u8 **pptr UNNEEDED, u32 v UNNEEDED) -{ fprintf(stderr, "towire_u32 called!\n"); abort(); } -/* Generated stub for towire_u64 */ -void towire_u64(u8 **pptr UNNEEDED, u64 v UNNEEDED) -{ fprintf(stderr, "towire_u64 called!\n"); abort(); } -/* Generated stub for towire_u8 */ -void towire_u8(u8 **pptr UNNEEDED, u8 v UNNEEDED) -{ fprintf(stderr, "towire_u8 called!\n"); abort(); } -/* Generated stub for towire_u8_array */ -void towire_u8_array(u8 **pptr UNNEEDED, const u8 *arr UNNEEDED, size_t num UNNEEDED) -{ fprintf(stderr, "towire_u8_array called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ int main(int argc, char *argv[]) @@ -287,6 +224,41 @@ int main(int argc, char *argv[]) expect->u.unresolved.port = 1234; assert(wireaddr_internal_eq(&addr, expect)); + /* + * Test for an out-of-bounds bug in fromwire_wireaddr(). + * + * This test reproduces a bug that occurs when decoding a DNS wireaddr + * of the maximum possible length (255 bytes). fromwire_wireaddr() + * would correctly read the 255 bytes of the address, but then attempt + * to write a null terminator at index 255, causing a one-byte + * overflow on the 255-byte destination buffer. + * + * The expected UBSan error is: + * common/wireaddr.c:51:3: runtime error: index 255 out of bounds for type 'u8[255]' + */ + + const char *dnsaddr = + /* 63 'a' */ "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa." + /* 63 'b' */ "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb." + /* 63 'c' */ "ccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccccc." + /* 63 'd' */ "ddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddddd"; + + struct wireaddr wa = { + .type = ADDR_TYPE_DNS, + .addrlen = 255, + .port = DEFAULT_PORT + }; + + memcpy(wa.addr, dnsaddr, wa.addrlen); + + u8 *encoded_wa = tal_arr(tmpctx, u8, 0); + towire_wireaddr(&encoded_wa, &wa); + size_t encoded_wa_len = tal_bytelen(encoded_wa); + + struct wireaddr decoded_wa; + assert(fromwire_wireaddr((const u8 **) &encoded_wa, &encoded_wa_len, &decoded_wa)); + assert(wireaddr_eq(&wa, &decoded_wa)); + tal_free(expect); common_shutdown(); } diff --git a/common/wireaddr.c b/common/wireaddr.c index 484dd830c857..80b8105b62eb 100644 --- a/common/wireaddr.c +++ b/common/wireaddr.c @@ -48,7 +48,6 @@ bool fromwire_wireaddr(const u8 **cursor, size_t *max, struct wireaddr *addr) case ADDR_TYPE_DNS: addr->addrlen = fromwire_u8(cursor, max); memset(&addr->addr, 0, sizeof(addr->addr)); - addr->addr[addr->addrlen] = 0; break; default: return false;