On Sat, Mar 18, 2023 at 01:10:49PM -0600, Todd C. Miller wrote:
> Thanks, I was unable to get a backtrace so this really helped. I
> think the safest thing to do is to just return an error if the
> expanded string is NULL. I'm not sure if there are other expansions
> that can also be NULL here.
>
> Alternately, we could move the check to be specific to the
> else if (!strcasecmp("mda", rtoken)) {
> ...
> }
>
> block.
I like your fix. It fixes the issue at hand. I tested this on my amd64
box.
However when I read the mda_variables.c code I wanted to go exceed a buffer,
originally I was almost convinced there was an off by one. The mda macro
thing stopped me short of completing my analysis because of the segfault.
Having the return there may have opened an avenue for that so what I'm
gonna play with it when I find some time next week, perhaps there is a
way to do more damage now that we return and not core.
Some facts, if I remember them right. LINE_BUF is 2048 bytes, the .forward
max size is 4096 bytes, and I think EXPAND_BUF is 128 bytes or so (here I
could be mistaken).
Thanks! Also thanks to Omar Polo who provided a great backtrace and debug.
I wish I would have done this to save time. But it makes us a great team!
Best Regards,
-peter
> - todd
>
> Index: mda_variables.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/smtpd/mda_variables.c,v
> retrieving revision 1.7
> diff -u -p -u -r1.7 mda_variables.c
> --- mda_variables.c 14 Jun 2021 17:58:15 -0000 1.7
> +++ mda_variables.c 18 Mar 2023 19:03:11 -0000
> @@ -51,7 +51,7 @@ mda_expand_token(char *dest, size_t len,
> {
> char rtoken[MAXTOKENLEN];
> char tmp[EXPAND_BUFFER];
> - const char *string;
> + const char *string = NULL;
> char *lbracket, *rbracket, *content, *sep, *mods;
> ssize_t i;
> ssize_t begoff, endoff;
> @@ -159,6 +159,8 @@ mda_expand_token(char *dest, size_t len,
> return -1;
>
> if (string != tmp) {
> + if (string == NULL)
> + return -1;
> if (strlcpy(tmp, string, sizeof tmp) >= sizeof tmp)
> return -1;
> string = tmp;
