Re: BUG: Using pointer that points to a destructed string's content
authorTamas Szakaly <sghctoma@gmail.com>
Fri, 26 Dec 2014 23:06:55 +0000 (00:06 +0100)
committerW. Trevor King <wking@tremily.us>
Sat, 20 Aug 2016 21:47:03 +0000 (14:47 -0700)
60/bd153eddad000aaa424e475ff12c3075c5093b [new file with mode: 0644]

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