NetBSD Problem Report #44390

From www@NetBSD.org  Fri Jan 14 19:59:41 2011
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id 266D463B883
	for <gnats-bugs@gnats.NetBSD.org>; Fri, 14 Jan 2011 19:59:41 +0000 (UTC)
Message-Id: <20110114195940.6E24D63B873@www.NetBSD.org>
Date: Fri, 14 Jan 2011 19:59:40 +0000 (UTC)
From: paul_koning@dell.com
Reply-To: paul_koning@dell.com
To: gnats-bugs@NetBSD.org
Subject: ftpd/ftpcmd.y inet6 case has out of range subscripts
X-Send-Pr-Version: www-1.0

>Number:         44390
>Category:       bin
>Synopsis:       ftpd/ftpcmd.y inet6 case has out of range subscripts
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    lukem
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Jan 14 20:00:00 +0000 2011
>Closed-Date:    Mon Apr 23 20:57:25 +0000 2012
>Last-Modified:  Tue Apr 24 18:30:03 +0000 2012
>Originator:     Paul Koning
>Release:        5.0.0
>Organization:
Dell
>Environment:
>Description:
When building 5.0 from source using GCC 4.5.1, I get some error messages due to new checks in the compiler.  Some have been reported before; this one apparently not.

libexec/ftpd/ftpcmd.y at line 979 and up loads data into a[8] and up.  But "a" is a cast to struct sockaddr_in6 of variable dest_addr which is struct sockinet.  

Apparently the out of bounds references are not seen as such by older compilers, they are fooled by the cast.  GCC 4.5.1 is not fooled and complains.

Obviously I can hide the error by more casting, but it seems to me that this is actually a memory overwrite error and that can't be a good thing.
>How-To-Repeat:
Build ftpd with a sufficiently new compiler
>Fix:

>Release-Note:

>Audit-Trail:
From: Paul Koning <paul_koning@dell.com>
To: <gnats-bugs@NetBSD.org>
Cc: <gnats-admin@netbsd.org>,
 <netbsd-bugs@netbsd.org>
Subject: Re: bin/44390: ftpd/ftpcmd.y inet6 case has out of range subscripts
Date: Fri, 14 Jan 2011 15:05:53 -0500

 That's weird, struct sockinet IS a union already and the code references =
 the sockaddr_in6 variant.  This may be a compiler bug instead.

 Looking further...

 	paul

From: Paul Koning <paul_koning@dell.com>
To: <gnats-bugs@NetBSD.org>
Cc: <gnats-admin@netbsd.org>,
 <netbsd-bugs@netbsd.org>
Subject: Re: bin/44390: ftpd/ftpcmd.y inet6 case has out of range subscripts
Date: Fri, 14 Jan 2011 15:53:16 -0500

 The attached patch cures the compiler complaint.  It isn't entirely =
 clear to me why this helps, but it seems like a useful change anyway =
 because it eliminates a cast.

 	paul

 Index: ftpcmd.y
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 --- ftpcmd.y	(revision 151799)
 +++ ftpcmd.y	(working copy)
 @@ -966,18 +966,31 @@
  		NUMBER
  		{
  #ifdef INET6
 -			char *a, *p;
 -
 +			char *p;
 +			struct in6_addr *a;
 +           =20
  			memset(&data_dest, 0, sizeof(data_dest));
  			data_dest.su_len =3D sizeof(struct =
 sockaddr_in6);
  			data_dest.su_family =3D AF_INET6;
  			p =3D (char *)&data_dest.su_port;
  			p[0] =3D $39.i; p[1] =3D $41.i;
 -			a =3D (char =
 *)&data_dest.si_su.su_sin6.sin6_addr;
 -			a[0] =3D $5.i; a[1] =3D $7.i; a[2] =3D $9.i; =
 a[3] =3D $11.i;
 -			a[4] =3D $13.i; a[5] =3D $15.i; a[6] =3D $17.i; =
 a[7] =3D $19.i;
 -			a[8] =3D $21.i; a[9] =3D $23.i; a[10] =3D $25.i; =
 a[11] =3D $27.i;
 -			a[12] =3D $29.i; a[13] =3D $31.i; a[14] =3D =
 $33.i; a[15] =3D $35.i;
 +			a =3D &data_dest.si_su.su_sin6.sin6_addr;
 +			a.s6_addr[0] =3D $5.i;
 +			a.s6_addr[1] =3D $7.i;=20
 +			a.s6_addr[2] =3D $9.i;
 +			a.s6_addr[3] =3D $11.i;
 +			a.s6_addr[4] =3D $13.i;
 +			a.s6_addr[5] =3D $15.i;
 +			a.s6_addr[6] =3D $17.i;
 +			a.s6_addr[7] =3D $19.i;
 +			a.s6_addr[8] =3D $21.i;
 +			a.s6_addr[9] =3D $23.i;
 +			a.s6_addr[10] =3D $25.i;=20
 +			a.s6_addr[11] =3D $27.i;
 +			a.s6_addr[12] =3D $29.i;
 +			a.s6_addr[13] =3D $31.i;
 +			a.s6_addr[14] =3D $33.i;
 +			a.s6_addr[15] =3D $35.i;
  			if (his_addr.su_family =3D=3D AF_INET6) {
  				/* XXX: more sanity checks! */
  				data_dest.su_scope_id =3D =
 his_addr.su_scope_id;

From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44390 CVS commit: src/libexec/ftpd
Date: Fri, 14 Jan 2011 18:56:13 -0500

 Module Name:	src
 Committed By:	christos
 Date:		Fri Jan 14 23:56:13 UTC 2011

 Modified Files:
 	src/libexec/ftpd: ftpcmd.y

 Log Message:
 PR/44390: Paul Koning: make code gcc-4.5.1 friendly.


 To generate a diff of this commit:
 cvs rdiff -u -r1.90 -r1.91 src/libexec/ftpd/ftpcmd.y

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Paul Koning <paul_koning@dell.com>
To: <gnats-bugs@NetBSD.org>
Cc: <gnats-admin@netbsd.org>,
 <netbsd-bugs@netbsd.org>
Subject: Re: PR/44390 CVS commit: src/libexec/ftpd
Date: Fri, 14 Jan 2011 20:56:19 -0500

 Um... oops...

 I sent the wrong diff.  I thought it built in my test build but I must =
 have overlooked something.  Need "a->" instead of "a.".  Attached is =
 better.

 	paul

 Index: libexec/ftpd/ftpcmd.y
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 --- libexec/ftpd/ftpcmd.y	(revision 151799)
 +++ libexec/ftpd/ftpcmd.y	(working copy)
 @@ -966,18 +966,31 @@
  		NUMBER
  		{
  #ifdef INET6
 -			char *a, *p;
 -
 +			char *p;
 +			struct in6_addr *a;
 +           =20
  			memset(&data_dest, 0, sizeof(data_dest));
  			data_dest.su_len =3D sizeof(struct =
 sockaddr_in6);
  			data_dest.su_family =3D AF_INET6;
  			p =3D (char *)&data_dest.su_port;
  			p[0] =3D $39.i; p[1] =3D $41.i;
 -			a =3D (char =
 *)&data_dest.si_su.su_sin6.sin6_addr;
 -			a[0] =3D $5.i; a[1] =3D $7.i; a[2] =3D $9.i; =
 a[3] =3D $11.i;
 -			a[4] =3D $13.i; a[5] =3D $15.i; a[6] =3D $17.i; =
 a[7] =3D $19.i;
 -			a[8] =3D $21.i; a[9] =3D $23.i; a[10] =3D $25.i; =
 a[11] =3D $27.i;
 -			a[12] =3D $29.i; a[13] =3D $31.i; a[14] =3D =
 $33.i; a[15] =3D $35.i;
 +			a =3D &data_dest.si_su.su_sin6.sin6_addr;
 +			a->s6_addr[0] =3D $5.i;
 +			a->s6_addr[1] =3D $7.i;=20
 +			a->s6_addr[2] =3D $9.i;
 +			a->s6_addr[3] =3D $11.i;
 +			a->s6_addr[4] =3D $13.i;
 +			a->s6_addr[5] =3D $15.i;
 +			a->s6_addr[6] =3D $17.i;
 +			a->s6_addr[7] =3D $19.i;
 +			a->s6_addr[8] =3D $21.i;
 +			a->s6_addr[9] =3D $23.i;
 +			a->s6_addr[10] =3D $25.i;=20
 +			a->s6_addr[11] =3D $27.i;
 +			a->s6_addr[12] =3D $29.i;
 +			a->s6_addr[13] =3D $31.i;
 +			a->s6_addr[14] =3D $33.i;
 +			a->s6_addr[15] =3D $35.i;
  			if (his_addr.su_family =3D=3D AF_INET6) {
  				/* XXX: more sanity checks! */
  				data_dest.su_scope_id =3D =
 his_addr.su_scope_id;

Responsible-Changed-From-To: bin-bug-people->lukem
Responsible-Changed-By: lukem@NetBSD.org
Responsible-Changed-When: Mon, 05 Sep 2011 09:53:46 +0000
Responsible-Changed-Why:
.


State-Changed-From-To: open->feedback
State-Changed-By: lukem@NetBSD.org
State-Changed-When: Mon, 05 Sep 2011 09:53:46 +0000
State-Changed-Why:
Paul, did Christos' fix in rev 1.91 fix the issue for you?


From: <Paul_Koning@Dell.com>
To: <gnats-bugs@NetBSD.org>
Cc: <gnats-admin@netbsd.org>, <netbsd-bugs@netbsd.org>
Subject: RE: PR/44390 CVS commit: src/libexec/ftpd
Date: Mon, 23 Apr 2012 20:34:12 +0000

 Much delayed response... yes, that fix is ok, it's a bit different from wha=
 t I first submitted and it cures  my mistake in a different way.

 	paul

State-Changed-From-To: feedback->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Mon, 23 Apr 2012 20:57:25 +0000
State-Changed-Why:
Confirmed ok.


From: Matthew Mondor <mm_lists@pulsar-zone.net>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: bin/44390 (ftpd/ftpcmd.y inet6 case has out of range
 subscripts)
Date: Tue, 24 Apr 2012 14:25:33 -0400

 > Synopsis: ftpd/ftpcmd.y inet6 case has out of range subscripts

 The fix was committed on -current yet the report was for netbsd-5, are
 any pullups needed?

 Thanks,
 -- 
 Matt

>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.