Re: Unchecked sprintf/strcat?
* Mon May 29 2006 Thomas Roessler <roessler@xxxxxxxxxxxxxxxxxx>
> On 2006-05-29 21:51:13 +0900, TAKAHASHI Tamotsu wrote:
> > From: TAKAHASHI Tamotsu <ttakah@xxxxxxxxxxxxxxxxx>
>
> > Because we have no way to detect overflow after safe_strcat,
> > I make safe_strcat return NULL when truncation occurs.
>
> Eeeeek. Don't do that.
>
> Return an int like in snprintf if all you want to do is know
> more about how much safe_strfcat copied. If you want to be
> able to use the result of safe_strfcat in other string
> operations, return a pointer to the destination string.
Good. I can simply use OpenBSD's strlcat.
(That needs a few lines in configure.ac.)
Anyway, the current code doesn't use the return value at all.
> > snprintf returns "the number of characters that would have been
> > output if the size were unlimited." - according to "man 3 snprintf."
> > So it's easy to detect overflow.
> >
> > strncpy can't do a good job. I just check strlen before strfcpy.
> > (Well, I haven't tried
> > "#define safe_strfcpy(A,B,C) *(A)=0;safe_strcat(A,C,B)")
>
> Eeeeek again.
>
> You basically doubled the runtime of safe_strfcpy. Also, the
> macro you did there isn't safe with respect to side effects.
> Don't do that.
So, Shall I use strlcpy?
> Also, please don't bother users with mutt_error output about
> truncated strings. When something goes wrong and indicates
> mutt gets into an undefined state, abort(). When the user
> needs to take action, do mutt_error().
>
> dprint may be appropriate in the places you use here, but I'm
> not at all convinced it's worth the effort.
True, such truncation is very rare.
But silent truncation makes debugging difficult.
The more dprint, the better.
> Frankly, if you want to improve string handling in mutt, move
> it away from fixed-size buffers completely. That's a better
> use of your time than trying to add error messages where
> fixed-size buffers are still used.
Do you mean:
strfcpy -> mutt_buffer_from
safe_strcat -> mutt_buffer_add
--
tamo