Re: SIGABRT when testing md5sum.cc

[Michael Goffioul]

On Tue, Nov 23, 2010 at 7:25 AM, John W. Eaton <address@hidden> wrote:
> On 22-Nov-2010, jkirby wrote:
>
> | I saw this crash on a msvc build. null-termination of sprintf not being
> | accounted for?
> |
> |
> | diff -r 521f2bb7c443 liboctave/oct-md5.cc
> | --- a/liboctave/oct-md5.cc    Thu Nov 18 20:14:52 2010 -0500
> | +++ b/liboctave/oct-md5.cc    Mon Nov 22 22:36:54 2010 -0500
> | @@ -36,7 +36,7 @@
> |  static std::string
> |  oct_md5_result_to_str (const unsigned char *buf)
> |  {
> | -  char tmp [32];
> | +     char tmp [32+1] = { 0 };
> |
> |    for (octave_idx_type i = 0; i < 16; i++)
> |      sprintf (&tmp[2*i], "%02x", buf[i]);
>
> I don't understand how that can matter.  The complete function is
>
>  static std::string
>  oct_md5_result_to_str (const unsigned char *buf)
>  {
>    char tmp [32];
>
>    for (octave_idx_type i = 0; i < 16; i++)
>      sprintf (&tmp[2*i], "%02x", buf[i]);
>
>    return std::string (tmp, 32);
>  }
>
> and since the std::string constructor includes the length of the array
> pointed to by tmp, it should not be going past the first 32 characters
> of the array.

Not the std::string constructor, but the sprintf call can get past the 32 chars
if you take into account the null character: when i = 15.

Michael.