[notmuch] [PATCH] format_part_json: part_content->data is not null terminated

[Gregor Hoffleit]

Both of you Davids are indeed completely right. Even more since the
next command in the patch after memcpy zeroes that byte.

This is how it's meant to be:

+    content_data = talloc_size (ctx, part_content->len+1);
+    memcpy (content_data, (char *)part_content->data, part_content->len);
+    content_data[part_content->len] = 0;

Should I submit a fixed patch?

Mea culpa,
    Gregor


* David Edmondson <dme at dme.org> [Do Apr 01 13:50:54 +0200 2010]
> On Thu, 01 Apr 2010 08:40:37 -0300, David Bremner <david at tethera.net> wrote:
> > On Thu, 04 Mar 2010 11:49:48 +0100, Gregor Hoffleit <gregor at hoffleit.de> wrote:
> > > In format_part_json, part_content->data is not a null terminated
> > > string.
> > 
> > I'd like to see this bug fixed,
> 
> +1.
> 
> > and the patch is pretty small, but...
> > 
> > > Instead, we have to use part_content->len.
> > > +    content_data = talloc_size (ctx, part_content->len+1);
> > > +    memcpy (content_data, (char *)part_content->data, part_content->len+1);
> > 
> > Can anyone explain why we copy (what seems to me to be) one extra byte
> > here?  In principle reading outside our allocated memory could cause
> > problems; at minimum it makes a false positive for a memory checker like
> > valgrind.
> 
> Agreed. It looks as though this should copy only part_content->len bytes.
> 
> dme.

-- 
Gregor Hoffleit <gregor.hoffleit at mediasupervision.de>
Media Supervision Software Consulting GmbH
Georg-Friedrich-Haendel-Str. 13, 69214 Eppelheim/Heidelberg
Tel: +49 6221 705079-22  /  Fax: +49 6221 705079-80
Amtsgericht Mannheim, HRB 336821, Geschäftsführer Reinhard Kratzke
http://www.mediasupervision.de/