[Spice-devel] [PATCH spice-server] red-stream-device: Fix "make syntax-check"

[Frediano Ziglio]

> Hi,
> 
> On Thu, 2019-01-31 at 11:44 +0100, Christophe Fergeau wrote:
> > On Wed, Jan 30, 2019 at 03:13:06PM +0000, Frediano Ziglio wrote:
> > > Avoid using strncpy, considered not secure.
> > > In this case a simple memcpy is used, we are going to terminate
> > > the string in any case on the next line.
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/red-stream-device.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/server/red-stream-device.c b/server/red-stream-device.c
> > > index 440b2689..2a210cc7 100644
> > > --- a/server/red-stream-device.c
> > > +++ b/server/red-stream-device.c
> > > @@ -330,9 +330,9 @@ handle_msg_device_display_info(StreamDevice *dev,
> > > SpiceCharDeviceInstance *sin)
> > >          return true;
> > >      }
> > >  
> > > -    strncpy(dev->device_display_info.device_address,
> > > -            (char*) display_info_msg->device_address,
> > > -            device_address_len);
> > > +    memcpy(dev->device_display_info.device_address,
> > > +           (char*) display_info_msg->device_address,
> > > +           device_address_len);
> > 
> > I'd use g_strlcpy instead, as by using memcpy, it's no longer obvious
> > that display_info_msg->device_address is not expected to contain null
> > bytes.
> 
> g_strlcpy requires the string to be terminated with a '\0', but you
> can't rely on that as the data come from the network. It would mean
> writing the zero to the received data buffer, which is not so nice.
> 
> Cheers,
> Lukas
> 
> > Christophe

You are both right... unfortunately.
As in the documentation g_strlcpy provide strlcpy replacement where
not available. However strlcpy is not portably guaranteeing that it does
not read more that dest_size bytes. Some does a kind of strlen before
and then limit the copy so if the source is not terminated you can
have a reading buffer overflow.

Frediano