From: YASUOKA Masahiko Subject: diff: isakmpd abort by freeing a wrong pointer To: tech@openbsd.org Date: Thu, 25 Dec 2025 18:19:57 +0900 Hello, As a summary, the diff fixes the problem isakmpd aborted by freeing a wrong pointer. The problem only happens on unusual setups. The problem happens when isakmpd receives an acquire message from the kernel, and it has a source/destination id formatted in prefix type, but it is not formed as isakmpd expected. 2777 dstid = memchr(dstident + 1, '/', slen); 2778 if (!dstid) { 2779 log_print("pf_key_v2_acquire: " 2780 "badly formatted PREFIX identity"); 2781 goto fail; 2782 } dstid is a middle of the buffer if it has '/'. 2783 masklen = atoi(dstid + 1); 2784 2785 /* XXX We only support host addresses. */ 2786 if ((afamily == AF_INET6 && masklen != 128) || 2787 (afamily == AF_INET && masklen != 32)) { 2788 log_print("pf_key_v2_acquire: " 2789 "non-host address specified in " 2790 "destination identity (mask length %d), " 2791 "ignoring request", masklen); 2792 goto fail; if it is not "/32" nor "/128", goto fail 2793 } (snip) 3169 fail: 3170 if (ret) 3171 pf_key_v2_msg_free(ret); 3172 if (askpolicy) 3173 pf_key_v2_msg_free(askpolicy); 3174 free(srcid); 3175 free(dstid); dstid is a middle the buffer. So the check in malloc.c fails. Usually, the kernel never sends such a message. isakmpd receives only acquire messages for the SAs created by isakmpd itself. But if iked could run beside isakmpd or an SA remains after iked terminated, this could actually happen. The SA registered by iked has a destination identity like "IPV4/192.168.1.101". The simplest fix for this, put dstid = NULL before #2729. But I'd like to propose the diff bellow which makes the checks simpler. ok? comments? Index: sbin/isakmpd/pf_key_v2.c =================================================================== RCS file: /var/cvs/openbsd/src/sbin/isakmpd/pf_key_v2.c,v diff -u -p -r1.205 pf_key_v2.c --- sbin/isakmpd/pf_key_v2.c 7 Aug 2023 04:01:30 -0000 1.205 +++ sbin/isakmpd/pf_key_v2.c 24 Dec 2025 08:36:08 -0000 @@ -2341,7 +2341,7 @@ pf_key_v2_acquire(struct pf_key_v2_msg * char dstbuf[ADDRESS_MAX], srcbuf[ADDRESS_MAX], *peer = 0; char confname[120], *conn = 0; char *srcid = 0, *dstid = 0, *prefstring = 0; - int slen, af, afamily, masklen; + int slen, af, afamily; struct sockaddr *smask, *sflow, *dmask, *dflow; struct sadb_protocol *sproto; char ssflow[ADDRESS_MAX], sdflow[ADDRESS_MAX]; @@ -2623,35 +2623,24 @@ pf_key_v2_acquire(struct pf_key_v2_msg * /* Check for valid type. */ switch (srcident->sadb_ident_type) { case SADB_IDENTTYPE_PREFIX: - /* Determine what the address family is. */ - srcid = memchr(srcident + 1, ':', slen); - if (srcid) - afamily = AF_INET6; - else + slen = strlen((char *)(srcident + 1)); + if (slen > 3 && strcmp((char *)(srcident + 1) + + slen - 3, "/32") == 0) { + *((char *)(srcident + 1) + slen - 3) = '\0'; afamily = AF_INET; - - srcid = memchr(srcident + 1, '/', slen); - if (!srcid) { + } else if (slen > 4 && strcmp((char *)(srcident + 1) + + slen - 4, "/128") == 0) { + *((char *)(srcident + 1) + slen - 4) = '\0'; + afamily = AF_INET6; + } else { log_print("pf_key_v2_acquire: " "badly formatted PREFIX identity"); goto fail; } - masklen = atoi(srcid + 1); - - /* XXX We only support host addresses. */ - if ((afamily == AF_INET6 && masklen != 128) || - (afamily == AF_INET && masklen != 32)) { - log_print("pf_key_v2_acquire: " - "non-host address specified in source " - "identity (mask length %d), ignoring " - "request", masklen); - goto fail; - } /* * NUL-terminate the PREFIX string at the separator, * then dup. */ - *srcid = '\0'; if (asprintf(&srcid, "id-%s", (char *) (srcident + 1)) == -1) { log_error("pf_key_v2_acquire: asprintf() failed"); @@ -2763,39 +2752,32 @@ pf_key_v2_acquire(struct pf_key_v2_msg * if (dstident) { slen = (dstident->sadb_ident_len * sizeof(u_int64_t)) - sizeof(struct sadb_ident); - + if (((unsigned char *) (dstident + 1))[slen - 1] != '\0') { + log_print("pf_key_v2_acquire: " + "destination identity not NUL-terminated"); + goto fail; + } /* Check for valid type. */ switch (dstident->sadb_ident_type) { case SADB_IDENTTYPE_PREFIX: - /* Determine what the address family is. */ - dstid = memchr(dstident + 1, ':', slen); - if (dstid) - afamily = AF_INET6; - else + slen = strlen((char *)(dstident + 1)); + if (slen > 3 && strcmp((char *)(dstident + 1) + + slen - 3, "/32") == 0) { + *((char *)(dstident + 1) + slen - 3) = '\0'; afamily = AF_INET; - - dstid = memchr(dstident + 1, '/', slen); - if (!dstid) { + } else if (slen > 4 && strcmp((char *)(dstident + 1) + + slen - 4, "/128") == 0) { + *((char *)(dstident + 1) + slen - 4) = '\0'; + afamily = AF_INET6; + } else { log_print("pf_key_v2_acquire: " "badly formatted PREFIX identity"); goto fail; } - masklen = atoi(dstid + 1); - - /* XXX We only support host addresses. */ - if ((afamily == AF_INET6 && masklen != 128) || - (afamily == AF_INET && masklen != 32)) { - log_print("pf_key_v2_acquire: " - "non-host address specified in " - "destination identity (mask length %d), " - "ignoring request", masklen); - goto fail; - } /* * NUL-terminate the PREFIX string at the separator, * then dup. */ - *dstid = '\0'; if (asprintf(&dstid, "id-%s", (char *) (dstident + 1)) == -1) { log_error("pf_key_v2_acquire: asprintf() failed");