From e1ee17d6fcc66a1b34e150c2fed9ad8e333318fb Mon Sep 17 00:00:00 2001 From: Tamas Szakaly Date: Sat, 27 Dec 2014 00:06:55 +0100 Subject: [PATCH] Re: BUG: Using pointer that points to a destructed string's content --- 60/bd153eddad000aaa424e475ff12c3075c5093b | 153 ++++++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 60/bd153eddad000aaa424e475ff12c3075c5093b diff --git a/60/bd153eddad000aaa424e475ff12c3075c5093b b/60/bd153eddad000aaa424e475ff12c3075c5093b new file mode 100644 index 000000000..6ac6b52de --- /dev/null +++ b/60/bd153eddad000aaa424e475ff12c3075c5093b @@ -0,0 +1,153 @@ +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 5AB73431FDB + for ; Fri, 26 Dec 2014 15:07:13 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.799 +X-Spam-Level: +X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5 + tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, + FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +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 iGnTMPkF3zrr for ; + Fri, 26 Dec 2014 15:07:09 -0800 (PST) +Received: from mail-wi0-f172.google.com (mail-wi0-f172.google.com + [209.85.212.172]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 8CD16431FAF + for ; Fri, 26 Dec 2014 15:07:09 -0800 (PST) +Received: by mail-wi0-f172.google.com with SMTP id n3so17843984wiv.11 + for ; Fri, 26 Dec 2014 15:07:08 -0800 (PST) +DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; + h=date:from:to:subject:message-id:mail-followup-to:references + :mime-version:content-type:in-reply-to:user-agent; + bh=IgSyy902JT5Kv7DSQSP7fItBj3DGy+WsYD12BRyuYmM=; + b=aN8KQct22KS+bXnMWanHgiDbaYiVpWc43QCSeV3tY3JolwuYbnYvijpSJ4Hg5pR4f8 + xCTvoLWiLNk3bexfwqKTr06SfxYzPdeK+8kyRHZx2SyrOJjrQn6+YO0GXZpbPhn7wuT+ + 460LDzCjCgXvm2Zh9tbuTRIpvtTj70SLMZojdoYp1IA8YmTCmrr7v615OzTA1X6fWojL + TwuWrmWYUw3HzVf0LHBB6YYqwiTlU16aXVcMG4sB7rIFaRepLVrpe35pWswwS9x3DSsM + 43iWoGFBLIHAbSY+ZwexbU5RPoH8y8UHn2Bv+m14+Nbfm+tSE5biXfXvam/hV9qldujn + Hvfw== +X-Received: by 10.180.83.42 with SMTP id n10mr43397993wiy.54.1419635228193; + Fri, 26 Dec 2014 15:07:08 -0800 (PST) +Received: from localhost (catv-37-191-19-235.catv.broadband.hu. + [37.191.19.235]) by mx.google.com with ESMTPSA id + j10sm30739722wjn.23.2014.12.26.15.07.04 for + (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); + Fri, 26 Dec 2014 15:07:07 -0800 (PST) +Date: Sat, 27 Dec 2014 00:06:55 +0100 +From: Tamas Szakaly +To: notmuch@notmuchmail.org +Subject: Re: BUG: Using pointer that points to a destructed string's content +Message-ID: <20141226230655.GA41992@pamparam> +Mail-Followup-To: notmuch@notmuchmail.org +References: <20141226113755.GA64154@pamparam> + <87oaqqf4ri.fsf@maritornes.cs.unb.ca> +MIME-Version: 1.0 +Content-Type: text/plain; charset=utf-8; x-action=pgp-signed +In-Reply-To: <87oaqqf4ri.fsf@maritornes.cs.unb.ca> +User-Agent: Mutt/1.5.23.1-rc1 (2014-03-12) +X-Mailman-Approved-At: Sat, 27 Dec 2014 07:45:39 -0800 +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: Fri, 26 Dec 2014 23:07:13 -0000 + +-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA1 + +> > The following line is from _notmuch_message_add_directory_terms in +> > lib/message.cc (line 652 in HEAD): +> > +> > direntry = (*i).c_str (); +> > +> > 'i' is a Xapian::TermIterator, whose operator* returns a std::string by value. +> > This means that c_str() is called on a temporary, which is destructed after the +> > full expression (essentially the particular line in this case), so 'direntry' +> > will point to a destructed std::string's data. +> > (See https://gcc.gnu.org/onlinedocs/gcc/Temporaries.html) +> +> Does the following patch fix it for you? I have to double check that +> direntry wasn't needed for something, but the test suite passes ;). +> +> diff --git a/lib/message.cc b/lib/message.cc +> index a7a13cc..24d0d5b 100644 +> --- a/lib/message.cc +> +++ b/lib/message.cc +> @@ -649,10 +649,8 @@ _notmuch_message_add_directory_terms (void *ctx, notmuch_message_t *message) +> /* Indicate that there are filenames remaining. */ +> status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID; +> +> - direntry = (*i).c_str (); +> - direntry += direntry_prefix_len; +> - +> - directory_id = strtol (direntry, &colon, 10); +> + directory_id = strtol ( +> + (*i).c_str () + direntry_prefix_len, &colon, 10); +> +> if (colon == NULL || *colon != ':') +> INTERNAL_ERROR ("malformed direntry"); +> + +Nope, it's still not correct: with this code "colon" will point into the +temporary string's data. I'm using this patch now: + +- --- work/notmuch-0.18.1/lib/message.cc 2014-06-25 12:30:10.000000000 +0200 ++++ /root/message.cc 2014-12-20 08:15:10.000000000 +0100 +@@ -601,18 +601,19 @@ + unsigned int directory_id; + const char *direntry, *directory; + char *colon; ++ const std::string& tmp = *i; + ++ direntry = tmp.c_str (); ++ + /* Terminate loop at first term without desired prefix. */ +- - if (strncmp ((*i).c_str (), direntry_prefix, direntry_prefix_len)) ++ if (strncmp (direntry, direntry_prefix, direntry_prefix_len)) + break; + + /* Indicate that there are filenames remaining. */ + status = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID; + +- - direntry = (*i).c_str (); +- - direntry += direntry_prefix_len; +- - +- - directory_id = strtol (direntry, &colon, 10); ++ directory_id = strtol ( ++ direntry + direntry_prefix_len, &colon, 10); + + if (colon == NULL || *colon != ':') + INTERNAL_ERROR ("malformed direntry"); + +This way operator* and c_str will be called only once, and its also better than +my first suggestion (using strdup), since it requires no additional memory +allocation. + +Regards, +sghctoma +- -- +-----BEGIN PGP SIGNATURE----- +Version: GnuPG v2 + +iQEcBAEBAgAGBQJUneoOAAoJEE8tbNCQOSmEzTwH/2NtrBberwi1nPdNgOLm2j9g +StrfYLnoIJIjn3dU7/X22QnX7CQHhXDMa+bY7UHDiS+oNTmMfRg0j6t1DV/tA/Pv +gA2Ti80zP8B1c0YX/KELGQYcxSQTE/p4hFe0Ff/Yo1Qi4KvFntxBiuvB6I1quQmX +1Kxew2l/Oq2PLOfqtmqfg5GuoDMXiNjNQgmfsV6x+i9F5PR3qnuvrzZvUWCC0URX +Xx2w1P+JYBambLdMqOH9MmU4ck/lobCkxprcfUK27i/LSNsl2UH+650bjef3Y6gR +CoErHJ4xso9+T8Dk+zRTtK1+uAs66xrLYRverJAnIL3LXFQG+czhUzuAdTaFQCM= +=PqvU +-----END PGP SIGNATURE----- -- 2.26.2