Re: [Qemu-devel] [PATCH] security_20040618

[Tim <tim-qemu@sentinelchicken.org>]

> > --- qemu-current/monitor.c	2004-06-16 20:49:59.000000000 -0700
> > +++ qemu-dev/monitor.c	2004-06-17 22:12:49.000000000 -0700
> >                  str = qemu_malloc(strlen(buf) + 1);
> > -                strcpy(str, buf);
> > +                pstrcpy(str, strlen(buf) + 1,  buf);
> 
> In my opinion, it already absolutely unnecessary correction.
> There is in this place no problem.

Yeah, you are probably right.  I looked at that one on 3 seperate
occasions before making the change, since I recognized that there are
very few conditions where it could possibly be a problem, and come to
think of it, this fix doesn't mitigate those conditions.

That chunk of code makes me uncomfortable for other reasons though (does
qemu_malloc() return NULL ever?  could buf possibly be missing a
trailing '\0' ever?) so I'll re-visit it again and see what makes the
most sense.  The pstrcpy isn't hurting anything though.  Slightly slower
copy, due to the length checking, but it isn't in a critical piece of
code (monitor.c is just for the user interface command prompt, right?),
so I also don't see a reason to remove it, esp if changes in the future
open up the possibility of an overflow.

thanks for the comment,
tim