From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7824 invoked by alias); 27 Nov 2013 18:07:29 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 32060 Received: (qmail 21638 invoked from network); 27 Nov 2013 18:07:23 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_HI, SPF_HELO_PASS autolearn=ham version=3.3.2 X-AuditID: cbfec7f4-b7fea6d0000026ac-29-529634d7954f Date: Wed, 27 Nov 2013 18:07:19 +0000 From: Peter Stephenson To: zsh-workers@zsh.org Subject: Re: PATCH: utils.c: Fix use of uninitialized memory in metafy(). Message-id: <20131127180719.1ad6acf0@pwslap01u.europe.root.pri> In-reply-to: References: Organization: Samsung Cambridge Solution Centre X-Mailer: Claws Mail 3.7.9 (GTK+ 2.22.0; i386-redhat-linux-gnu) MIME-version: 1.0 Content-type: text/plain; charset=US-ASCII Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFuphluLIzCtJLcpLzFFi42I5/e/4Fd3rJtOCDG5/N7Q42PyQyYHRY9XB D0wBjFFcNimpOZllqUX6dglcGat/NDAW9HBWdF78wdjAuIi9i5GTQ0LAROLF8wOsELaYxIV7 69m6GLk4hASWMkpMWrGXCcJZziTxqvsTWAeLgKrE/xN32UBsNgFDiambZjOC2CIC4hJn155n AbGFBTwlTm1/ywxi8wrYSzxZ/5YJxOYUiJY4vPgnUD0H0NAoiTPHE0HC/AL6Elf/fmKCOMJe YuaVM4wQrYISPybfAxvJLKAlsXlbEyuELS+xec1b5gmMArOQlM1CUjYLSdkCRuZVjKKppckF xUnpuYZ6xYm5xaV56XrJ+bmbGCEh+GUH4+JjVocYBTgYlXh4D2hOCxJiTSwrrsw9xCjBwawk whumAxTiTUmsrEotyo8vKs1JLT7EyMTBKdXAWFR1zFL3SvCmaVlM6xzrFLnkOJblXGx4Un5R 88+7eYvm+BctlPq9dlF4cxJzfErra8s3cg1bLPeujxHrSnwdlnWuXGGN8kHpKq7ts1u8tr08 s+W55JO7vSvu22/Ytsdumavz/ew47pSy1ywdOycf++eyfE38U7Xzp05deKJ48PjyOKZnTXYh 55VYijMSDbWYi4oTARvQ3UwfAgAA On Wed, 27 Nov 2013 18:45:16 +0100 Simon Ruderich wrote: > While running the tests with valgrind I noticed an use of > uninitialized memory in metafy(). > > The following patch should fix it, but I don't know the details > of this code, so please check it before applying the patch. > > The problem is the *e != '\0' in the next if, once e == buf + > len, *e points after buf. Hmm... I think the intention probably *is* to check if there's null termination at "buf + len", on the assumption that the first "len" bytes need metafying regardless. So if we've got only len valid bytes, not null-terminated (or null-terminated by accident because the next byte that isn't actually valid for the allocation happens to be null), we've got no way of knowing this given the current interface. That's not actually stated explicitly but the comment above does mention len+1 for copying, implying len doesn't include the termination. It looks like either we've got to improve the interface, which is a lot of work, or always copy when we're give a length, which is inefficient. I'd be tempted to do the latter for now. pws