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

Gregor Hoffleit gregor at hoffleit.de
Thu Apr 1 05:05:47 PDT 2010


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/


More information about the notmuch mailing list