Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

bin/44390: ftpd/ftpcmd.y inet6 case has out of range subscripts

0 views
Skip to first unread message

paul_...@dell.com

unread,
Jan 14, 2011, 3:00:01 PM1/14/11
to
>Number: 44390
>Category: bin
>Synopsis: ftpd/ftpcmd.y inet6 case has out of range subscripts
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: bin-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Fri Jan 14 20:00:00 +0000 2011
>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:


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-...@muc.de

Paul Koning

unread,
Jan 14, 2011, 3:05:53 PM1/14/11
to
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

Paul Koning

unread,
Jan 14, 2011, 3:10:06 PM1/14/11
to
The following reply was made to PR bin/44390; it has been noted by GNATS.

From: Paul Koning <paul_...@dell.com>
To: <gnats...@NetBSD.org>
Cc: <gnats...@netbsd.org>,
<netbs...@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 =

Paul Koning

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

Paul Koning

unread,
Jan 14, 2011, 4:30:06 PM1/14/11
to
The following reply was made to PR bin/44390; it has been noted by GNATS.

From: Paul Koning <paul_...@dell.com>
To: <gnats...@NetBSD.org>
Cc: <gnats...@netbsd.org>,
<netbs...@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 =

Christos Zoulas

unread,
Jan 14, 2011, 7:00:06 PM1/14/11
to
The following reply was made to PR bin/44390; it has been noted by GNATS.

From: "Christos Zoulas" <chri...@netbsd.org>
To: gnats...@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.

Paul Koning

unread,
Jan 14, 2011, 8:56:19 PM1/14/11
to
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
===================================================================
--- 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;
+

memset(&data_dest, 0, sizeof(data_dest));
data_dest.su_len = sizeof(struct sockaddr_in6);
data_dest.su_family = AF_INET6;
p = (char *)&data_dest.su_port;
p[0] = $39.i; p[1] = $41.i;
- a = (char *)&data_dest.si_su.su_sin6.sin6_addr;
- a[0] = $5.i; a[1] = $7.i; a[2] = $9.i; a[3] = $11.i;
- a[4] = $13.i; a[5] = $15.i; a[6] = $17.i; a[7] = $19.i;
- a[8] = $21.i; a[9] = $23.i; a[10] = $25.i; a[11] = $27.i;
- a[12] = $29.i; a[13] = $31.i; a[14] = $33.i; a[15] = $35.i;
+ a = &data_dest.si_su.su_sin6.sin6_addr;

+ a->s6_addr[0] = $5.i;
+ a->s6_addr[1] = $7.i;
+ a->s6_addr[2] = $9.i;
+ a->s6_addr[3] = $11.i;
+ a->s6_addr[4] = $13.i;
+ a->s6_addr[5] = $15.i;
+ a->s6_addr[6] = $17.i;
+ a->s6_addr[7] = $19.i;
+ a->s6_addr[8] = $21.i;
+ a->s6_addr[9] = $23.i;
+ a->s6_addr[10] = $25.i;
+ a->s6_addr[11] = $27.i;
+ a->s6_addr[12] = $29.i;
+ a->s6_addr[13] = $31.i;
+ a->s6_addr[14] = $33.i;
+ a->s6_addr[15] = $35.i;
if (his_addr.su_family == AF_INET6) {


/* XXX: more sanity checks! */

data_dest.su_scope_id = his_addr.su_scope_id;

Paul Koning

unread,
Jan 14, 2011, 9:00:06 PM1/14/11
to
The following reply was made to PR bin/44390; it has been noted by GNATS.

From: Paul Koning <paul_...@dell.com>


To: <gnats...@NetBSD.org>
Cc: <gnats...@netbsd.org>,
<netbs...@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 =

Message has been deleted
Message has been deleted
Message has been deleted
Message has been deleted
0 new messages