Re: Unchecked sprintf/strcat?
* TAKAHASHI Tamotsu [06-05-24 20:12:28 +0900] wrote:
safe_* will abort() to dump core if overflowed.
I think this is really bad for the "main" reason that the implementation
of these "safe" functions cannot know how to deal with an error
properly. Also, in general it's now always due to a too small buffer if
an action fails because some buffer sizes make sense and thus have to be
For example, when a user enters a >_POSIX_PATH_MAX filename, calling
abort() and telling him the internal buffer was too small is simply
wrong. His filename was too long and thus it's his error, not mutt's.
I think there're many more examples like this and if we want to go
further in securing the code, we have to do two things (IMHO):
1. Don't handle the errors in the wrapper functions but move it out
to calling code as much as possible because only the caller can know
what to do in case of an error. An a general abort() is just wrong.
2. Really put these into a header and make them available inline. I
didn't do any profiling yet, but at a point where nearly every libc
call is wrapped up by a sanity wrapper, mutt could become slower by a
I don't really see a problem with the code right now because it mostly
ensures there're no buffer overflows (and thus remote code injection and
all that) making the OS call abort(). We should instead just check
errors more consequently, perhaps.