The current code for extracting the token name from %{name} can be simplified by computing the token name length. The existing code copies "name}" to token[] using memcpy(), then strchr() to find the '}' and replace it with a NUL. Using strchr() here is fragile since token[] is not yet NUL-terminated. This is currently not a problem since there is an earlier check for '}' in the source string but it could be dangerous is the code changes further.
I find it much simpler to compute the token name length, verify the length, copy the bytes and then explicitly NUL-terminate token. This results in less code and is more easily audited. I've also removed the duplicate check for *(pbuf+1) != '{'. OK? - todd Index: usr.sbin/smtpd/mda_variables.c =================================================================== RCS file: /cvs/src/usr.sbin/smtpd/mda_variables.c,v retrieving revision 1.8 diff -u -p -U10 -u -r1.8 mda_variables.c --- usr.sbin/smtpd/mda_variables.c 19 Mar 2023 01:43:11 -0000 1.8 +++ usr.sbin/smtpd/mda_variables.c 19 Mar 2023 13:59:01 -0000 @@ -236,21 +236,21 @@ mda_expand_token(char *dest, size_t len, ssize_t mda_expand_format(char *buf, size_t len, const struct deliver *dlv, const struct userinfo *ui, const char *mda_command) { char tmpbuf[EXPAND_BUFFER], *ptmp, *pbuf, *ebuf; char exptok[EXPAND_BUFFER]; ssize_t exptoklen; char token[MAXTOKENLEN]; - size_t ret, tmpret; + size_t ret, tmpret, toklen; if (len < sizeof tmpbuf) { log_warnx("mda_expand_format: tmp buffer < rule buffer"); return -1; } memset(tmpbuf, 0, sizeof tmpbuf); pbuf = buf; ptmp = tmpbuf; ret = tmpret = 0; @@ -261,48 +261,46 @@ mda_expand_format(char *buf, size_t len, if (tmpret >= sizeof tmpbuf) { log_warnx("warn: user directory for %s too large", ui->directory); return 0; } ret += tmpret; ptmp += tmpret; pbuf += 2; } - /* expansion loop */ for (; *pbuf && ret < sizeof tmpbuf; ret += tmpret) { if (*pbuf == '%' && *(pbuf + 1) == '%') { *ptmp++ = *pbuf++; pbuf += 1; tmpret = 1; continue; } if (*pbuf != '%' || *(pbuf + 1) != '{') { *ptmp++ = *pbuf++; tmpret = 1; continue; } /* %{...} otherwise fail */ - if (*(pbuf+1) != '{' || (ebuf = strchr(pbuf+1, '}')) == NULL) + if ((ebuf = strchr(pbuf+2, '}')) == NULL) return 0; /* extract token from %{token} */ - if ((size_t)(ebuf - pbuf) - 1 >= sizeof token) + toklen = ebuf - (pbuf+2); + if (toklen >= sizeof token) return 0; - memcpy(token, pbuf+2, ebuf-pbuf-1); - if (strchr(token, '}') == NULL) - return 0; - *strchr(token, '}') = '\0'; + memcpy(token, pbuf+2, toklen); + token[toklen] = '\0'; exptoklen = mda_expand_token(exptok, sizeof exptok, token, dlv, ui, mda_command); if (exptoklen == -1) return -1; /* writing expanded token at ptmp will overflow tmpbuf */ if (sizeof (tmpbuf) - (ptmp - tmpbuf) <= (size_t)exptoklen) return -1;