bin/151664: [PATCH] sbin/route/route.c: Incorrect array bounds checking

[Alexey Illarionov]

>Number:         151664
>Category:       bin
>Synopsis:       [PATCH] sbin/route/route.c: Incorrect array bounds checking
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Sat Oct 23 16:40:04 UTC 2010
>Closed-Date:
>Last-Modified:
>Originator:     Alexey Illarionov
>Release:        8.1-STABLE
>Organization:
>Environment:
8.1-STABLE
>Description:
sbin/route/route.c have incorrect bounds checking of msgtypes[] in print_rtmsg():

char *msgtypes[] = {
 "",
 ...
 0
};

void
print_rtmsg(rtm, msglen)
{
..
   if (msgtypes[rtm->rtm_type] != NULL)
       (void)printf("%s: ", msgtypes[rtm->rtm_type]);
..
}

There is also no checks for received message length (msglen) there.

>How-To-Repeat:
Run `route monitor` and send invalid message to PF_ROUTE socket:

$ route monitor &
[1] 13682
$ perl -MSocket  -e 'socket(SOCK, PF_ROUTE, SOCK_RAW, 0); syswrite(SOCK, pack("Scc",4,5,0xa0));'

got message of size 4 on Sat Oct 23 20:26:51 2010
[1]+  Segmentation fault: 11  route monitor

>Fix:


Patch attached with submission follows:

--- route.c.orig	2010-10-23 19:33:31.560869646 +0400
+++ route.c	2010-10-23 20:28:18.947314302 +0400
@@ -1303,7 +1303,7 @@
 	"RTM_NEWMADDR: new multicast group membership on iface",
 	"RTM_DELMADDR: multicast group membership removed from iface",
 	"RTM_IFANNOUNCE: interface arrival/departure",
-	0,
+	"RTM_IEEE80211: IEEE80211 wireless event"
 };
 
 char metricnames[] =
@@ -1341,7 +1341,7 @@
 		    rtm->rtm_version);
 		return;
 	}
-	if (msgtypes[rtm->rtm_type] != NULL)
+	if (rtm->rtm_type < sizeof(msgtypes)/sizeof(msgtypes[0]))
 		(void)printf("%s: ", msgtypes[rtm->rtm_type]);
 	else
 		(void)printf("#%d: ", rtm->rtm_type);
@@ -1349,6 +1349,10 @@
 	switch (rtm->rtm_type) {
 	case RTM_IFINFO:
 		ifm = (struct if_msghdr *)rtm;
+		if (msglen < sizeof(struct if_msghdr)) {
+		   printf("invalid\n");
+		   break;
+		}
 		(void) printf("if# %d, ", ifm->ifm_index);
 		switch (ifm->ifm_data.ifi_link_state) {
 		case LINK_STATE_DOWN:
@@ -1368,6 +1372,10 @@
 	case RTM_NEWADDR:
 	case RTM_DELADDR:
 		ifam = (struct ifa_msghdr *)rtm;
+		if (msglen < sizeof(struct ifa_msghdr)) {
+		   printf("invalid\n");
+		   break;
+		}
 		(void) printf("metric %d, flags:", ifam->ifam_metric);
 		bprintf(stdout, ifam->ifam_flags, routeflags);
 		pmsg_addrs((char *)(ifam + 1), ifam->ifam_addrs);
@@ -1376,11 +1384,19 @@
 	case RTM_NEWMADDR:
 	case RTM_DELMADDR:
 		ifmam = (struct ifma_msghdr *)rtm;
+		if (msglen < sizeof(struct ifma_msghdr)) {
+		   printf("invalid\n");
+		   break;
+		}
 		pmsg_addrs((char *)(ifmam + 1), ifmam->ifmam_addrs);
 		break;
 #endif
 	case RTM_IFANNOUNCE:
 		ifan = (struct if_announcemsghdr *)rtm;
+		if (msglen < sizeof(struct if_announcemsghdr)) {
+		   printf("invalid\n");
+		   break;
+		}
 		(void) printf("if# %d, what: ", ifan->ifan_index);
 		switch (ifan->ifan_what) {
 		case IFAN_ARRIVAL:
@@ -1397,6 +1413,10 @@
 		break;
 
 	default:
+		if (msglen < sizeof(struct if_msghdr)){
+		   printf("invalid\n");
+		   break;
+		}
 		(void) printf("pid: %ld, seq %d, errno %d, flags:",
 			(long)rtm->rtm_pid, rtm->rtm_seq, rtm->rtm_errno);
 		bprintf(stdout, rtm->rtm_flags, routeflags);


>Release-Note:
>Audit-Trail:
>Unformatted: