[Spice-devel] [PATCH vd_agent_win] Use sprintf_s instead of sprintf to not crash

[Christophe Fergeau]

On Wed, May 17, 2017 at 12:57:34PM -0400, Frediano Ziglio wrote:
> > 
> > > 
> > > ---
> > >  vdagent/file_xfer.cpp | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/vdagent/file_xfer.cpp b/vdagent/file_xfer.cpp
> > > index de1aea1..de98d50 100644
> > > --- a/vdagent/file_xfer.cpp
> > > +++ b/vdagent/file_xfer.cpp
> > > @@ -113,8 +113,8 @@ void
> > > FileXfer::handle_start(VDAgentFileXferStartMessage*
> > > start,
> > >          if (attempt == 0) {
> > >              strcpy(dest_filename, file_name);
> > >          } else {
> > > -            sprintf(dest_filename, "%.*s (%d)%s", int(extension -
> > > file_name), file_name,
> > > -                    attempt, extension);
> > > +            sprintf_s(dest_filename, SPICE_N_ELEMENTS(file_name) +
> > > POSTFIX_LEN,
> > > +                      "%.*s (%d)%s", int(extension - file_name),
> > > file_name,
> > > attempt, extension);
> > >          }
> > >          if ((MultiByteToWideChar(CP_UTF8, 0, dest_filename, -1, file_path
> > >          +
> > >          wlen, MAX_PATH - wlen)) == 0) {
> > >              vd_printf("failed converting file_name:%s to WideChar",
> > >              dest_filename);
> > 
> > As far as I know dest_filename is allocate big enough to avoid the
> > overflow.
> > Did it crash for you or just a warning from the compiler?
> > 
> > This would be easier
> > 
> >             sprintf_s(dest_filename, SPICE_N_ELEMENTS(dest_filename),
> >                       "%.*s (%d)%s", int(extension - file_name), file_name,
> >                       attempt, extension);
> > 
> > without computing again the length.
> > 
> 
> Got some discussion with Jakub and Pavel on IRC.
> They confirmed that with Fedora 26 and 27 the crash happens.
> I tried to do some tests (removed the CreateFile and some calls to have
> a test function) with Fedora 25 but didn't manage to reproduce.
> Jakub reported that this happens with small names.
> 
> Maybe is an issue with MingW with newer Fedora? Worth investigating.

This is pushed already, but this should have been added to the commit
log.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170523/43555ddb/attachment.sig>