Re: Unchecked sprintf/strcat?
* Sun May 21 2006 Oswald Buddenhagen <ossi@xxxxxxx>
> On Sun, May 21, 2006 at 06:35:46PM +0900, TAKAHASHI Tamotsu wrote:
> > I've found a few unchecked sprintf and strcat.
> > They don't look very dangerous, but you may want to fix them.
> >
> i find this sort of paranoid. snprintf & co are for strings with unknown
> length (where i don't like them too much as well, as they cause silent
> truncation which is often hard to debug).
You are right. Mutt uses sprintf/strcat properly in most cases.
And they are marked as /* __STRCAT_CHECKED__ */.
What I meant was I found a few lines lacking the mark.
You can put the marks instead of using safe_strcat.
But the sizeof(dest)<sizeof(src) cases are marked, and possible
to overflow. Bugs like this can happen if someone increases
the size of src after checked. safe_strcat protects the code
from such modification. So safe_* series are useful.
> if you don't trust the own code, use functions that raise an assertion
> failure when the buffer is overflowed.
Thanks for the suggestion.
I'd rather add the assertion to safe_strcat and simply use
"safe_strcat(dst,sizeof(dst),buf)" than use
"if(len>0&&len<sizeof(buf))strcat(dst,len);else return(1);"
everywhere.
--
tamo
--- lib.c~ Sat May 20 22:52:30 2006
+++ lib.c Mon May 22 11:16:09 2006
@@ -230,8 +230,16 @@ char *safe_strcat (char *d, size_t l, co
for (; *d && l; l--)
d++;
+#ifdef DEBUG
+ if (!l)
+ dprint (2, (debugfile, "safe_strcat: dest is already full.\n"));
+#endif
for (; *s && l; l--)
*d++ = *s++;
+#ifdef DEBUG
+ if (!l)
+ dprint (2, (debugfile, "safe_strcat: dest is full. truncated.\n"));
+#endif
*d = '\0';