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

Frediano Ziglio fziglio at redhat.com
Wed May 17 16:57:34 UTC 2017


> 
> > 
> > ---
> >  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.

For me sprintf_s is fine. I would just replace the
"SPICE_N_ELEMENTS(file_name) + POSTFIX_LEN" with SPICE_N_ELEMENTS(dest_filename)

Frediano


More information about the Spice-devel mailing list