Return-Path: X-Original-To: notmuch@notmuchmail.org Delivered-To: notmuch@notmuchmail.org Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id C16554196F0 for ; Thu, 1 Apr 2010 05:05:50 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.001 X-Spam-Level: X-Spam-Status: No, score=-0.001 tagged_above=-999 required=5 tests=[BAYES_40=-0.001] autolearn=ham Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8fipop0AQ+Al for ; Thu, 1 Apr 2010 05:05:49 -0700 (PDT) Received: from sam.mediasupervision.de (sam.mediasupervision.de [80.152.3.104]) by olra.theworths.org (Postfix) with ESMTP id 86248431FC1 for ; Thu, 1 Apr 2010 05:05:49 -0700 (PDT) Received: from localhost (sam.mediasupervision.de [127.0.0.1]) by sam.mediasupervision.de (Postfix) with ESMTP id 787A0485C6D; Thu, 1 Apr 2010 14:05:48 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at sam.mediasupervision.de Received: from sam.mediasupervision.de ([127.0.0.1]) by localhost (sam.mediasupervision.de [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 8h+Wpi3fSBf8; Thu, 1 Apr 2010 14:05:48 +0200 (CEST) Received: by sam.mediasupervision.de (Postfix, from userid 1000) id 4F760485DDE; Thu, 1 Apr 2010 14:05:48 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 From: Gregor Hoffleit To: David Edmondson In-reply-to: <87634bjsrl.fsf@ut.hh.sledj.net> References: <1267699753-sup-3919@sam.mediasupervision.de> <871vezs8ne.fsf@rocinante.cs.unb.ca> <87634bjsrl.fsf@ut.hh.sledj.net> Date: Thu, 01 Apr 2010 14:05:47 +0200 Message-Id: <1270123286-sup-8763@sam.mediasupervision.de> User-Agent: Sup/git Content-Transfer-Encoding: 8bit Cc: notmuch Subject: Re: [notmuch] [PATCH] format_part_json: part_content->data is not null terminated X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 01 Apr 2010 12:05:50 -0000 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 [Do Apr 01 13:50:54 +0200 2010] > On Thu, 01 Apr 2010 08:40:37 -0300, David Bremner wrote: > > On Thu, 04 Mar 2010 11:49:48 +0100, Gregor Hoffleit 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 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/