Re: isakmpd: Validate DELETE payload SPI array size 1/2

hshoexer <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
Hi,

anyone?

On Wed, Apr 22, 2026 at 03:02:17PM +0200, hshoexer wrote:
> Hi,
> 
> The number of SPIs provided in a DELETE message is not properly
> validated. This might cause a read beyond the message end. However,
> the outside read is limited to 4 bytes for IPsec SAs and 16 bytes
> for ISAKMP SAs. A crash is possible, but seems unlikely to me.
> 
> ok?
> 
> Take care,
> HJ.
> 
> diff --git a/sbin/isakmpd/message.c b/sbin/isakmpd/message.c
> index c49a14d67b7..41392ca7f41 100644
> --- a/sbin/isakmpd/message.c
> +++ b/sbin/isakmpd/message.c
> @@ -620,6 +620,7 @@ message_validate_delete(struct message *msg, struct payload *p)
>  	struct doi     *doi;
>  	struct sa      *sa, *isakmp_sa;
>  	struct sockaddr *dst, *dst_isa;
> +	size_t		spisz, len;
>  	u_int32_t       nspis = GET_ISAKMP_DELETE_NSPIS(p->p);
>  	u_int8_t       *spis = (u_int8_t *)p->p + ISAKMP_DELETE_SPI_OFF;
>  	u_int32_t       i;
> @@ -666,6 +667,24 @@ message_validate_delete(struct message *msg, struct payload *p)
>  		message_free(msg);
>  		return -1;
>  	}
> +	/* Enforce that SPI size matches the protocol. */
> +	spisz = (proto == ISAKMP_PROTO_ISAKMP) ?
> +	    ISAKMP_HDR_COOKIES_LEN : sizeof(u_int32_t);
> +	if (GET_ISAKMP_DELETE_SPI_SZ(p->p) != spisz) {
> +		log_print("message_validate_delete: invalid SPI size %u "
> +		    "for protocol %u", GET_ISAKMP_DELETE_SPI_SZ(p->p), proto);
> +		message_free(msg);
> +		return -1;
> +	}
> +	/* All SPIs must fit into the DELETE payload. */
> +	len = GET_ISAKMP_GEN_LENGTH(p->p);
> +	if (len < ISAKMP_DELETE_SPI_OFF ||
> +	    (len - ISAKMP_DELETE_SPI_OFF) / spisz < nspis) {
> +		log_print("message_validate_delete: SPI count %u exceeds "
> +		    "payload length %zu", nspis, len);
> +		message_free(msg);
> +		return -1;
> +	}
>  	/* Validate the SPIs.  */
>  	for (i = 0; i < nspis; i++) {
>  		/* Get ISAKMP SA protecting this message. */
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.