Merge pull request #953 from jbonor/nhrpd-fixes

nhrpd: fix issues found by coverity

nhrpd/linux.c

@@ -105,7 +105,7 @@ static int linux_configure_arp(const char *iface, int on)
 {
 	struct ifreq ifr;
 
-	strncpy(ifr.ifr_name, iface, IFNAMSIZ);
+	strncpy(ifr.ifr_name, iface, IFNAMSIZ - 1);
 	if (ioctl(nhrp_socket_fd, SIOCGIFFLAGS, &ifr))
 		return -1;
 

nhrpd/netlink.h

@@ -15,7 +15,7 @@ struct interface;
 extern int netlink_nflog_group;
 extern int netlink_req_fd;
 
-int netlink_init(void);
+void netlink_init(void);
 int netlink_configure_arp(unsigned int ifindex, int pf);
 void netlink_update_binding(struct interface *ifp, union sockunion *proto, union sockunion *nbma);
 void netlink_set_nflog_group(int nlgroup);

nhrpd/netlink_arp.c

@@ -230,20 +230,27 @@ void netlink_set_nflog_group(int nlgroup)
 	netlink_nflog_group = nlgroup;
 	if (nlgroup) {
 		netlink_log_fd = znl_open(NETLINK_NETFILTER,  0);
+		if (netlink_log_fd < 0)
+			return;
+
 		netlink_log_register(netlink_log_fd, nlgroup);
 		thread_add_read(master, netlink_log_recv, 0, netlink_log_fd,
 				&netlink_log_thread);
 	}
 }
 
-int netlink_init(void)
+void netlink_init(void)
 {
 	netlink_req_fd = znl_open(NETLINK_ROUTE, 0);
+	if (netlink_req_fd < 0)
+		return;
+
 	netlink_listen_fd = znl_open(NETLINK_ROUTE, RTMGRP_NEIGH);
+	if (netlink_listen_fd < 0)
+		return;
+
 	thread_add_read(master, netlink_route_recv, 0, netlink_listen_fd,
 			NULL);
-
-	return 0;
 }
 
 int netlink_configure_arp(unsigned int ifindex, int pf)

nhrpd/nhrp_event.c

@@ -59,8 +59,10 @@ static void evmgr_recv_message(struct event_manager *evmgr, struct zbuf *zb)
 		buf[len] = 0;
 
 		debugf(NHRP_DEBUG_EVENT, "evmgr: msg: %s", buf);
-		sscanf(buf, "eventid=%d", &eventid);
+		if (sscanf(buf, "eventid=%d", &eventid) != 1)
-		sscanf(buf, "result=%63s", result);
+			continue;
+		if (sscanf(buf, "result=%63s", result) != 1)
+			continue;
 	}
 	debugf(NHRP_DEBUG_EVENT, "evmgr: received: eventid=%d result=%s", eventid, result);
 	if (eventid && result[0]) {

nhrpd/nhrp_peer.c

@@ -598,6 +598,10 @@ static struct {
 	const char *name;
 	void (*handler)(struct nhrp_packet_parser *);
 } packet_types[] = {
+	[0] = {
+		.type = PACKET_UNKNOWN,
+		.name = "UNKNOWN",
+	},
 	[NHRP_PACKET_RESOLUTION_REQUEST] = {
 		.type = PACKET_REQUEST,
 		.name = "Resolution-Request",
@@ -797,7 +801,7 @@ void nhrp_peer_recv(struct nhrp_peer *p, struct zbuf *zb)
 
 	nbma_afi = htons(hdr->afnum);
 	proto_afi = proto2afi(htons(hdr->protocol_type));
-	if (hdr->type > ZEBRA_NUM_OF(packet_types) ||
+	if (hdr->type > NHRP_PACKET_MAX ||
 	    hdr->version != NHRP_VERSION_RFC2332 ||
 	    nbma_afi >= AFI_MAX || proto_afi == AF_UNSPEC ||
 	    packet_types[hdr->type].type == PACKET_UNKNOWN ||

nhrpd/nhrp_protocol.h

@@ -26,6 +26,7 @@
 #define NHRP_PACKET_PURGE_REPLY			6
 #define NHRP_PACKET_ERROR_INDICATION		7
 #define NHRP_PACKET_TRAFFIC_INDICATION		8
+#define NHRP_PACKET_MAX				8
 
 /* NHRP Extension Types */
 #define NHRP_EXTENSION_FLAG_COMPULSORY		0x8000

nhrpd/nhrp_vty.c

@@ -74,7 +74,7 @@ static int nhrp_vty_return(struct vty *vty, int ret)
 	if (ret == NHRP_OK)
 		return CMD_SUCCESS;
 
-	if (ret > 0 && ret <= (int)ZEBRA_NUM_OF(errmsgs))
+	if (ret > 0 && ret <= NHRP_ERR_MAX)
 		if (errmsgs[ret])
 			str = errmsgs[ret];
 

nhrpd/nhrpd.h

@@ -35,7 +35,9 @@ enum {
 	NHRP_ERR_ENTRY_EXISTS,
 	NHRP_ERR_ENTRY_NOT_FOUND,
 	NHRP_ERR_PROTOCOL_ADDRESS_MISMATCH,
+	__NHRP_ERR_MAX
 };
+#define NHRP_ERR_MAX		(__NHRP_ERR_MAX - 1)
 
 struct notifier_block;
 

nhrpd/vici.c

@@ -27,13 +27,13 @@ struct blob {
 
 static int blob_equal(const struct blob *b, const char *str)
 {
-	if (b->len != (int) strlen(str)) return 0;
+	if (!b || b->len != (int) strlen(str)) return 0;
 	return memcmp(b->ptr, str, b->len) == 0;
 }
 
 static int blob2buf(const struct blob *b, char *buf, size_t n)
 {
-	if (b->len >= (int) n) return 0;
+	if (!b || b->len >= (int) n) return 0;
 	memcpy(buf, b->ptr, b->len);
 	buf[b->len] = 0;
 	return 1;
@@ -82,8 +82,8 @@ static void vici_parse_message(
 	struct vici_message_ctx *ctx)
 {
 	uint8_t *type;
-	struct blob key;
+	struct blob key = { 0 };
-	struct blob val;
+	struct blob val = { 0 };
 
 	while ((type = zbuf_may_pull(msg, uint8_t)) != NULL) {
 		switch (*type) {
@@ -178,11 +178,15 @@ static void parse_sa_message(
 		}
 		break;
 	default:
+		if (!key)
+			break;
+
 		switch (key->ptr[0]) {
 		case 'l':
 			if (blob_equal(key, "local-host") && ctx->nsections == 1) {
 				if (blob2buf(val, buf, sizeof(buf)))
-					str2sockunion(buf, &sactx->local.host);
+					if (str2sockunion(buf, &sactx->local.host) < 0)
+						zlog_err("VICI: bad strongSwan local-host: %s", buf);
 			} else if (blob_equal(key, "local-id") && ctx->nsections == 1) {
 				sactx->local.id = *val;
 			} else if (blob_equal(key, "local-cert-data") && ctx->nsections == 1) {
@@ -192,7 +196,8 @@ static void parse_sa_message(
 		case 'r':
 			if (blob_equal(key, "remote-host") && ctx->nsections == 1) {
 				if (blob2buf(val, buf, sizeof(buf)))
-					str2sockunion(buf, &sactx->remote.host);
+					if (str2sockunion(buf, &sactx->remote.host) < 0)
+						zlog_err("VICI: bad strongSwan remote-host: %s", buf);
 			} else if (blob_equal(key, "remote-id") && ctx->nsections == 1) {
 				sactx->remote.id = *val;
 			} else if (blob_equal(key, "remote-cert-data") && ctx->nsections == 1) {
@@ -261,6 +266,7 @@ static void vici_recv_message(struct vici_conn *vici, struct zbuf *msg)
 	uint32_t msglen;
 	uint8_t msgtype;
 	struct blob name;
+	struct vici_message_ctx ctx;
 
 	msglen = zbuf_get_be32(msg);
 	msgtype = zbuf_get8(msg);
@@ -283,7 +289,7 @@ static void vici_recv_message(struct vici_conn *vici, struct zbuf *msg)
 			vici_recv_sa(vici, msg, 2);
 		break;
 	case VICI_CMD_RESPONSE:
-		vici_parse_message(vici, msg, parse_cmd_response, 0);
+		vici_parse_message(vici, msg, parse_cmd_response, &ctx);
 		break;
 	case VICI_EVENT_UNKNOWN:
 	case VICI_CMD_UNKNOWN:
@@ -381,8 +387,6 @@ static void vici_submit_request(struct vici_conn *vici, const char *name, ...)
 			zbuf_put_be16(obuf, len);
 			zbuf_put(obuf, va_arg(va, void *), len);
 			break;
-		case VICI_END:
-			break;
 		default:
 			break;
 		}
@@ -491,7 +495,7 @@ int sock_open_unix(const char *path)
 
 	memset(&addr, 0, sizeof (struct sockaddr_un));
 	addr.sun_family = AF_UNIX;
-	strncpy(addr.sun_path, path, strlen (path));
+	strncpy(addr.sun_path, path, sizeof(addr.sun_path) - 1);
 
 	ret = connect(fd, (struct sockaddr *) &addr, sizeof(addr.sun_family) + strlen(addr.sun_path));
 	if (ret < 0) {
@@ -499,7 +503,11 @@ int sock_open_unix(const char *path)
 		return -1;
 	}
 
-	fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK);
+	ret = fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK);
+	if (ret < 0) {
+		close(fd);
+		return -1;
+	}
 
 	return fd;
 }

nhrpd/znl.c

@@ -141,8 +141,10 @@ int znl_open(int protocol, int groups)
 	if (fd < 0)
 		return -1;
 
-	fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK);
+	if (fcntl(fd, F_SETFL, fcntl(fd, F_GETFL, 0) | O_NONBLOCK) < 0)
-	fcntl(fd, F_SETFD, FD_CLOEXEC);
+		goto error;
+	if (fcntl(fd, F_SETFD, FD_CLOEXEC) < 0)
+		goto error;
 	if (setsockopt(fd, SOL_SOCKET, SO_RCVBUF, &buf, sizeof(buf)) < 0)
 		goto error;