On Fri, Sep 15, 2017 at 09:43:58AM -0700, Kevin J. McCarthy wrote: > On Fri, Sep 15, 2017 at 12:58:18AM +0200, Eike Rathke wrote: > > This is nothing overly critical (not even in mutt_rmtree() if > > a truncated string also represents an existing directory entry, as there > > was a checked opendir(path) and all entries are to be removed, but > > truncated will not be removed as intended, so..), just a heads-up that > > some long paths may get truncated when concatenating into a buffer. > > Places that do such concatenation using snprintf() should check the > > return value and if greater or equal than size(...) it means the result > > is truncated. Maybe path related destination buffers also shouldn't be > > of _POSIX_PATH_MAX but PATH_MAX size instead. > > Thanks Eike. I'll put this on my list to look at when I have some > time. (I've been crazy busy recently). > > In general, this is a string I'm not too anxious to pull on (pun > intended), because Mutt uses fixed sized buffers for a lot of this > stuff. Sometimes the truncation is bad, but there are a few instances > where truncation is just fine and even planned for in the code.
*Unexpected* truncation is always bad. It simply means what the programmer expected and what actually is happening diverge, and that is always a bug. > It looks like Mutt uses _POSIX_PATH_MAX in the majority of cases with > just a sprinkle of PATH_MAX uses. Is PATH_MAX generally larger? PATH_MAX is highly system dependent, and in many cases, also completely bogus. Relying on it is often folly, but it's also often all you really can do. Here are a couple of nice blog posts about the issue, so that I don't have to try to reproduce the whole argument myself in this thread: =8^) http://insanecoding.blogspot.com/2007/11/pathmax-simply-isnt.html https://eklitzke.org/path-max-is-tricky The bottom line is, if the code doesn't check for truncation, it may well be a security problem. You may end up operating on a path that is not what the user specified, which is clearly a bug. This is just a variation of the strncpy() problem... BUT, at least sprintf() can be checked, whereas strncpy() always truncates silently, so sprintf() is the better choice. -- Derek D. Martin http://www.pizzashack.org/ GPG Key ID: 0xDFBEAD02 -=-=-=-=- This message is posted from an invalid address. Replying to it will result in undeliverable mail due to spam prevention. Sorry for the inconvenience.
pgpcFAyl1kgPk.pgp
Description: PGP signature
