Skip to content

common/wireaddr: Fix an out-of-bounds bug in the address parser #8325

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions common/test/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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/%)
98 changes: 35 additions & 63 deletions common/test/run-wireaddr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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[])
Expand Down Expand Up @@ -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();
}
1 change: 0 additions & 1 deletion common/wireaddr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down