Hi, * 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 enforced.
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 measurable factor.
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.
bye, Rocco -- :wq!