diff --git a/lightningd/peer_control.c b/lightningd/peer_control.c index 2b7875981..4d59121db 100644 --- a/lightningd/peer_control.c +++ b/lightningd/peer_control.c @@ -2530,11 +2530,6 @@ void peer_fundee_open(struct peer *peer, const u8 *from_peer, msg = towire_opening_fundee(peer, peer->minimum_depth, 7500, 150000, from_peer); - /* Careful here! Their message could push us overlength! */ - if (tal_len(msg) >= 65536) { - peer_fail_permanent_str(peer, "Unacceptably long open_channel"); - return; - } subd_req(peer, peer->owner, take(msg), -1, 2, opening_fundee_finished, peer); } diff --git a/wire/wire_io.c b/wire/wire_io.c index a0c2aff97..4e58b9da7 100644 --- a/wire/wire_io.c +++ b/wire/wire_io.c @@ -1,4 +1,3 @@ -#include /* FIXME: io_plan needs size_t */ #include #include @@ -6,6 +5,7 @@ #include #include #include +#include #include /* @@ -14,11 +14,11 @@ * scratch data, and it's almost enough for our purposes. */ -/* 2 bytes for the length header. */ -#define HEADER_LEN (sizeof(le16)) +/* 4 bytes for the length header. */ +#define HEADER_LEN (sizeof(wire_len_t)) -/* Since length can only be 64k, this is an impossible value. */ -#define INSIDE_HEADER_BIT 0x80000000 +/* We carefully never allow sizes > 64M, so this is an impossible value. */ +#define INSIDE_HEADER_BIT WIRE_LEN_LIMIT /* arg->u2.s contains length we've read, arg->u1.vp contains u8 **data. */ static int do_read_wire_header(int fd, struct io_plan_arg *arg) @@ -32,9 +32,13 @@ static int do_read_wire_header(int fd, struct io_plan_arg *arg) return -1; arg->u2.s += ret; - /* Both bytes read? Set up for normal read of data. */ + /* Length bytes read? Set up for normal read of data. */ if (arg->u2.s == INSIDE_HEADER_BIT + HEADER_LEN) { - arg->u2.s = be16_to_cpu(*(be16 *)p); + arg->u2.s = *(wire_len_t *)p; + if (arg->u2.s >= INSIDE_HEADER_BIT) { + errno = E2BIG; + return -1; + } /* A type-only message is not unheard of, so optimize a little */ if (arg->u2.s != HEADER_LEN) tal_resize((u8 **)arg->u1.vp, arg->u2.s); @@ -84,7 +88,7 @@ static int do_write_wire_header(int fd, struct io_plan_arg *arg) { ssize_t ret; size_t len = arg->u2.s & ~INSIDE_HEADER_BIT; - be16 hdr = cpu_to_be16(tal_count(arg->u1.const_vp)); + wire_len_t hdr = tal_count(arg->u1.const_vp); ret = write(fd, (char *)&hdr + len, HEADER_LEN - len); if (ret <= 0) @@ -128,6 +132,11 @@ struct io_plan *io_write_wire_(struct io_conn *conn, { struct io_plan_arg *arg = io_plan_arg(conn, IO_OUT); + if (tal_len(data) >= INSIDE_HEADER_BIT) { + errno = E2BIG; + return io_close(conn); + } + arg->u1.const_vp = tal_dup_arr(conn, u8, memcheck(data, tal_len(data)), tal_len(data), 0); diff --git a/wire/wire_io.h b/wire/wire_io.h index de9eb13b7..b381ece5e 100644 --- a/wire/wire_io.h +++ b/wire/wire_io.h @@ -4,6 +4,11 @@ #include #include +/* We don't allow > 64M msgs: enough for 483 64k failure msgs. */ +#define WIRE_LEN_LIMIT (1 << 26) + +typedef u32 wire_len_t; + /* Read message into *data, allocating off ctx. */ struct io_plan *io_read_wire_(struct io_conn *conn, const tal_t *ctx, diff --git a/wire/wire_sync.c b/wire/wire_sync.c index da8ed6834..0dc65af7b 100644 --- a/wire/wire_sync.c +++ b/wire/wire_sync.c @@ -1,16 +1,18 @@ -#include "wire/wire_sync.h" #include #include #include +#include +#include +#include bool wire_sync_write(int fd, const void *msg TAKES) { - be16 be_len = cpu_to_be16(tal_count(msg)); + wire_len_t len = tal_len(msg); bool ret; - assert(be16_to_cpu(be_len) == tal_count(msg)); - ret = write_all(fd, &be_len, sizeof(be_len)) - && write_all(fd, msg, tal_count(msg)); + assert(tal_len(msg) < WIRE_LEN_LIMIT); + ret = write_all(fd, &len, sizeof(len)) + && write_all(fd, msg, len); if (taken(msg)) tal_free(msg); @@ -19,13 +21,17 @@ bool wire_sync_write(int fd, const void *msg TAKES) u8 *wire_sync_read(const tal_t *ctx, int fd) { - be16 be_len; + wire_len_t len; u8 *msg; - if (!read_all(fd, &be_len, sizeof(be_len))) + if (!read_all(fd, &len, sizeof(len))) return NULL; - msg = tal_arr(ctx, u8, be16_to_cpu(be_len)); - if (!read_all(fd, msg, be16_to_cpu(be_len))) + if (len >= WIRE_LEN_LIMIT) { + errno = E2BIG; + return NULL; + } + msg = tal_arr(ctx, u8, len); + if (!read_all(fd, msg, len)) return tal_free(msg); return msg; }