Re: [PATCH vd_agent_win] Use sprintf_s instead of sprintf to not crash

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> > 
> > ---
> >  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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]