From owner-freebsd-security  Thu Aug  1  3:33:34 2002
Delivered-To: freebsd-security@freebsd.org
Received: from mx1.FreeBSD.org (mx1.FreeBSD.org [216.136.204.125])
	by hub.freebsd.org (Postfix) with ESMTP id 0A9F337B400
	for <freebsd-security@FreeBSD.ORG>; Thu,  1 Aug 2002 03:33:32 -0700 (PDT)
Received: from ptserver.progtech.net (pD9590BDA.dip.t-dialin.net [217.89.11.218])
	by mx1.FreeBSD.org (Postfix) with ESMTP id 53CEB43E6A
	for <freebsd-security@FreeBSD.ORG>; Thu,  1 Aug 2002 03:33:30 -0700 (PDT)
	(envelope-from rg@progtech.net)
Received: from PROGTECH.net (isis.muc.progtech.intern [10.25.0.100])
	by ptserver.progtech.net (8.12.3/8.12.3) with ESMTP id g71AXQuF070020
	for <freebsd-security@FreeBSD.ORG>; Thu, 1 Aug 2002 12:33:27 +0200 (CEST)
	(envelope-from rg@PROGTECH.net)
Message-ID: <3D490E77.6050003@PROGTECH.net>
Date: Thu, 01 Aug 2002 12:33:27 +0200
From: Rolf Grossmann <rg@progtech.net>
User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.1b) Gecko/20020729
X-Accept-Language: en,German [de]
MIME-Version: 1.0
To: freebsd-security@FreeBSD.ORG
Subject: Re: FreeBSD Security Advisory FreeBSD-SA-02:34.rpc
References: <200208010246.g712k6NM003336@freefall.freebsd.org>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Sender: owner-freebsd-security@FreeBSD.ORG
Precedence: bulk
List-ID: <freebsd-security.FreeBSD.ORG>
List-Archive: <http://docs.freebsd.org/mail/> (Web Archive)
List-Help: <mailto:majordomo@FreeBSD.ORG?subject=help> (List Instructions)
List-Subscribe: <mailto:majordomo@FreeBSD.ORG?subject=subscribe%20freebsd-security>
List-Unsubscribe: <mailto:majordomo@FreeBSD.ORG?subject=unsubscribe%20freebsd-security>
X-Loop: FreeBSD.org

Hi,

I've been looking at your patch for the rpc buffer overflow and I 
believe it's not sufficient.

You're using:

if ((c > maxsize && UINT_MAX/elsize < c) &&
    (xdrs->x_op != XDR_FREE)) {
	return (FALSE);

but I think it really should test both conditions (the braces suggest that you
actually meant that anyway):

if ((c > maxsize || UINT_MAX/elsize < c) &&
    (xdrs->x_op != XDR_FREE)) {
	return (FALSE);

Otherwise, if the writer of the application using xdr_array specified 
maxsize too large (maybe he didn't care), you're in trouble again. I think
it's clearer if you write the condition the other way round:

if ((c > maxsize || c > UINT_MAX/elsize) &&
    (xdrs->x_op != XDR_FREE)) {
	return (FALSE);

Should I file a PR or am I completely off track here?

Bye, Rolf




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