From owner-cvs-all  Thu Jan  4 11:20:34 2001
From owner-cvs-all@FreeBSD.ORG  Thu Jan  4 11:20:32 2001
Return-Path: <owner-cvs-all@FreeBSD.ORG>
Delivered-To: cvs-all@freebsd.org
Received: from gw.nectar.com (gw.nectar.com [208.42.49.153])
	by hub.freebsd.org (Postfix) with ESMTP
	id C163637B402; Thu,  4 Jan 2001 11:20:31 -0800 (PST)
Received: from hamlet.nectar.com (hamlet.nectar.com [10.0.1.102])
	by gw.nectar.com (Postfix) with ESMTP
	id AACA3193E2; Thu,  4 Jan 2001 13:20:30 -0600 (CST)
Received: (from nectar@localhost)
	by hamlet.nectar.com (8.11.1/8.9.3) id f04JKUk85713;
	Thu, 4 Jan 2001 13:20:30 -0600 (CST)
	(envelope-from nectar@spawn.nectar.com)
Date: Thu, 4 Jan 2001 13:20:30 -0600
From: "Jacques A. Vidrine" <n@nectar.com>
To: Will Andrews <will@FreeBSD.org>
Cc: cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org
Subject: Re: cvs commit: src/usr.bin/apply apply.c
Message-ID: <20010104132030.C85350@hamlet.nectar.com>
Mail-Followup-To: "Jacques A. Vidrine" <n@nectar.com>,
	Will Andrews <will@FreeBSD.org>, cvs-committers@FreeBSD.org,
	cvs-all@FreeBSD.org
References: <200101041905.f04J5ou82617@freefall.freebsd.org>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
In-Reply-To: <200101041905.f04J5ou82617@freefall.freebsd.org>; from will@FreeBSD.org on Thu, Jan 04, 2001 at 11:05:49AM -0800
X-Url: http://www.nectar.com/
Sender: nectar@nectar.com
Sender: owner-cvs-all@FreeBSD.ORG
Precedence: bulk
X-Loop: FreeBSD.ORG

On Thu, Jan 04, 2001 at 11:05:49AM -0800, Will Andrews wrote:
> will        2001/01/04 11:05:49 PST
> 
>   Modified files:
>     usr.bin/apply        apply.c 
>   Log:
>   BDECFLAGS; de-__P()-ify, ANSIfy, use snprintf() instead of sprintf(),
>   especially on strings passed from argv; rename system() to exec_shell(),
>   and make it static; use strlcpy() and make sure it works; use proper
>   type (size_t) to be passed to malloc()/realloc().
>   
>   Use getusershell() to make sure the SHELL environment variable passed is
>   safe to use.  Add new option -s to allow anal users to pass things like
>   perl; this option is here along with getusershell() checking since the
>   such checking is only intended to affect things like suidperl that might
>   call apply(1).
>   
>   Reviewed by:	markm, jhb, C. Stephen Gunn <csg@waterspout.com>
>   
>   Revision  Changes    Path
>   1.11      +41 -24    src/usr.bin/apply/apply.c

Please back this out.  You've introduced some new problems:

Around line 130 you have:

   p += snprintf(cmd, sizeof(cmd), "exec %s", argv[0]);

and

   p += snprintf(p, sizeof(p), " %c%d", magic, i);

These are bogus ... sizeof(cmd) == size of a pointer, not the buffer
to which it points.  So particularly for the first thing, `cmd' will
always be { 'e', 'x', 'e', '\0' } on i386.

The same mistake is seen around lines 141, 171, and 193.

Also, if you are going to use snprintf/strlcpy, you should check for
truncation in most cases.

Was this code run?  I don't see how it could have worked.
-- 
Jacques Vidrine / n@nectar.com / jvidrine@verio.net / nectar@FreeBSD.org


To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe cvs-all" in the body of the message