Re: [Nmh-workers] Nmh-commits Digest, Vol 108, Issue 11

[Ralph Corderoy]

Hi M,

> even pcc can handle C99 these days! :-)

That's nice.  And Arch has it in its AUR repository.  I've added it to
the list of build variations to try and set up someday.

> I've seen only one problem when using nmh compiled with pcc, and I
> think it's legitimately a bug in nmh rather than a problem with pcc.

Agreed, thanks.

> The fmt_scan() function in sbr/fmt_scan.c relies on undefined behavior
> when it calls strncpy(3) here:
>
>         case FT_LS_UNQUOTE:
>             if (str) {          
>                 if (str != buffer)
>                     strncpy(buffer, str, sizeof(buffer));
>
> That ought to be using memmove(3) rather than strncpy(3) because
> (depending on the message) it's possible that buffer and str can be
> overlapping strings if the FT_LS_TRIM case happened to be applicable:
>
>         case FT_LS_TRIM:
>             if (str) {
>                     char *xp;
>
>                     if (str != buffer)
>                         strncpy(buffer, str, sizeof(buffer));
>                     buffer[sizeof(buffer)-1] = '\0';
>                     str = buffer;
>                     while (isspace((unsigned char) *str))
>                             str++;

Printing strncpy()'s parameters, and the delta between the addresses,
does show an overlap with a gap of just one.

    $ scan -forma '%(unquote(trim{from}))' .
    trim 0x7fff7a907b60 0x55e66dff80d0 8192 0xffffd5e6f36f0570
    unqu 0x7fff7a907b60 0x7fff7a907b61 8192 0x1

The addition of `%(unquote)' with e8635a8a was faulty, but using
`%(trim)' twice could have also trigger it, and that pre-dates git.

Fix to be committed shortly, though I fear a valgrind suppression might
be needed to stop its `Source and destination overlap in memcpy_chk'
that's called by memmove(3)!  I used memmove(), but not instead of those
strncpy(3)s as they have different semantics I didn't want to change (or
fix).  Instead, if str might have wandered away from buffer[0] or
buffer2[0] then it's memmove'd back.  A comment for str also states that
requirement.

-- 
Cheers, Ralph.
https://plus.google.com/+RalphCorderoy