On 2016-09-05 19:07:18 -0500, Derek Martin wrote:
> On Mon, Sep 05, 2016 at 10:32:40AM -0500, Derek Martin wrote:
> > Is strfcpy() widely available?
>
> Ah, now I see that strfcpy() is a Mutt-specific macro that intends to
> make strncpy() safer. I was actually thinking of strlcpy(), which is
> equivalent to Mutt's strfcpy(); but it does not matter. ALL of these
> functions suffer from the same affliction: If dest is too small, they
> all silently lose data on copy.
The strfcpy macro looks awful:
# define strfcpy(A,B,C) strncpy(A,B,C), *(A+(C)-1)=0
First, I would add parentheses:
# define strfcpy(A,B,C) (...)
to make sure that it will always be seen as a whole, whatever the
context. Moreover, if A is not of type char *, it will silently
corrupt the memory. There should be a diagnostic (warning) from
the compiler, but this is usually non-fatal, so that warnings are
generally ignored.
> There are approximately 360 such calls to strfcpy() in Mut's code; if
> any of these calls are used in such a way that data loss is sensitive,
> it could result in a security exploit. It would require an audit.
>
> Maybe there's an easier way to deal with that problem that's still
> reasonable:
>
> #include <assert.h>
>
> int safe_strncpy(char *dest, char *src, size_t size)
> {
> return snprintf(dest, size, "%s", src);
> }
>
> /* guard against silent data loss on string copy */
> #define strfcpy(A,B,C) { int rc = safe_strcpy(A,B,C); assert(rc >= 0 && rc <
> (C)); }
Adding do ... while (0) would avoid some compilation issues:
#define strfcpy(A,B,C) \
do { int rc = safe_strcpy(A,B,C); assert(rc >= 0 && rc < (C)); } \
while (0)
This way, one can do:
if (...)
strfcpy (...);
else
...
> The abort on failure is annoying, but better than a potential security
> hole caused by silently truncating sensitive data, and the abort
> mostly shouldn't ever happen. But Mutt already uses assert() fairly
> liberally, and if there ARE bugs that trigger this, the above will
> make them easy to identify and fix. =8^)
I completely agree.
BTW, I even build and use Mutt with GCC's UB sanitizer
(-fsanitize=undefined -fno-sanitize-recover).
--
Vincent Lefèvre <[email protected]> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
