Re: [Qemu-devel] [PATCH] Security house-cleaning

[Gianni Tedesco]

On Thu, 2004-06-17 at 09:05 -0700, Tim wrote:
> > Thats only worrisome from a security perspective if qemu was designed to
> > run SUID, which I doubt that it is... Of course it's a bug and needs
> > fixing though.
> 
> Yes, I agree on both points.  There is little I can offer to this
> project right now, besides testing, and general code cleanup.  I know
> almost nothing about hardware emulation, so just trying to help out with
> what I know...

cool :)

> > What would be more worrying is if there were overflows in the packet
> > processing allowing (possibly compromised) guest OS or remote machines
> > to take over qemu process by sending an exploit in a malformed packet.
> 
> Agreed.  The slirp code in particular worries me in this respect.

I'll stick it on my todo list to have a look around in there when i'm
bored.

> > A quick note on the patch: where you are replacing strcpy() with
> > strncpy(), you are better to use snprintf(buf, sizeof(buf), "%s",
> > input); as that guarantees nul termination. It also allows you to easily
> > check if input was truncated, in some cases, silent truncation could be
> > a bug.
> 
> Ahh, good point.  However, if you specify a size one less than the size
> of your buffer, I believe strncpy fills the rest of your buffer w/
> nulls, doesn't it?  Or is that OS-specific?  So far, I have been keeping
> my size in strncpy() one less than the buffer size (see the 256->255
> change on one particular buffer, or instance).  However, this method is
> prone to off-by-one bugs, so I might switch to snprintf() in some cases,
> as you suggest.

nooooope, strcpy has no way of knowing how big the buffer is other than
what you tell it. It's likely that all (or most) of the buffers that are
strcpy'd to are initialised to zero / .bss so it doesn't matter in
reality, but better safe than sorry. What if some buffer is moved to
stack later, that would expose the latent bug.

-- 
// Gianni Tedesco (gianni at scaramanga dot co dot uk)
lynx --source www.scaramanga.co.uk/scaramanga.asc | gpg --import
8646BE7D: 6D9F 2287 870E A2C9 8F60 3A3C 91B5 7669 8646 BE7D

signature.asc
Description: This is a digitally signed message part