From: Matt Caswell Date: Wed, 21 Sep 2016 10:20:18 +0000 (+0100) Subject: Add warning about a potential pitfall with WPACKET_allocate_bytes() X-Git-Tag: OpenSSL_1_1_1-pre1~3465 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=4b0fc9fc7a8767f3e6289b2b9f4527db186b3566 Add warning about a potential pitfall with WPACKET_allocate_bytes() If the underlying BUF_MEM gets realloc'd then the pointer returned could become invalid. Therefore we should always ensure that the allocated memory is filled in prior to any more WPACKET_* calls. Reviewed-by: Rich Salz --- diff --git a/ssl/packet.c b/ssl/packet.c index 6199469969..0e8e8764dd 100644 --- a/ssl/packet.c +++ b/ssl/packet.c @@ -224,6 +224,7 @@ int WPACKET_start_sub_packet_len__(WPACKET *pkt, size_t lenbytes) if (!WPACKET_allocate_bytes(pkt, lenbytes, &lenchars)) return 0; + /* Convert to an offset in case the underlying BUF_MEM gets realloc'd */ sub->packet_len = lenchars - (unsigned char *)pkt->buf->data; return 1; diff --git a/ssl/packet_locl.h b/ssl/packet_locl.h index c51d8922a8..44a8f82c7c 100644 --- a/ssl/packet_locl.h +++ b/ssl/packet_locl.h @@ -671,6 +671,9 @@ int WPACKET_start_sub_packet(WPACKET *pkt); * Allocate bytes in the WPACKET for the output. This reserves the bytes * and counts them as "written", but doesn't actually do the writing. A pointer * to the allocated bytes is stored in |*allocbytes|. + * WARNING: the allocated bytes must be filled in immediately, without further + * WPACKET_* calls. If not then the underlying buffer may be realloc'd and + * change its location. */ int WPACKET_allocate_bytes(WPACKET *pkt, size_t bytes, unsigned char **allocbytes); @@ -715,7 +718,7 @@ int WPACKET_put_bytes__(WPACKET *pkt, unsigned int val, size_t bytes); #define WPACKET_put_bytes_u16(pkt, val) \ WPACKET_put_bytes__((pkt), (val), 2) #define WPACKET_put_bytes_u24(pkt, val) \ - WPACKET_put_bytes__((pkt), (val)), 3) + WPACKET_put_bytes__((pkt), (val), 3) #define WPACKET_put_bytes_u32(pkt, val) \ WPACKET_sub_allocate_bytes__((pkt), (val), 4)