netdev - [iproute PATCH] ip-route: Propagate errors from parse_one

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20180123164047.28661-1-phil@nwl.cc>
Date:   Tue, 23 Jan 2018 17:40:47 +0100
From:   Phil Sutter <phil@....cc>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     netdev@...r.kernel.org
Subject: [iproute PATCH] ip-route: Propagate errors from parse_one_nh()

The following command segfaults if enp0s31f6 does not exist:

| # ip -6 route add default proto ra metric 20100 \
| 	nexthop via fe80:52:0:2040::1fc dev enp0s31f6 weight 1 \
| 	nexthop via fe80:52:0:2040::1fe dev enp0s31f6 weight 1

Since the non-zero return code from parse_one_nh() is ignored,
parse_nexthops() continues iterating over the the same fields in argv
until buffer space is exhausted and eventually accesses unallocated
memory.

Fix this by aborting on error in parse_nexthops() and make
iproute_modify() fail if parse_nexthops() did.

Reported-by: Lennart Poettering <lpoetter@...hat.com>
Fixes: 2f406f2d0b4ef ("ip route: replace exits with returns")
Signed-off-by: Phil Sutter <phil@....cc>
---
 ip/iproute.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/ip/iproute.c b/ip/iproute.c
index bf886fda9d761..d7accf57ac8d1 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -871,7 +871,8 @@ static int parse_nexthops(struct nlmsghdr *n, struct rtmsg *r,
 		memset(rtnh, 0, sizeof(*rtnh));
 		rtnh->rtnh_len = sizeof(*rtnh);
 		rta->rta_len += rtnh->rtnh_len;
-		parse_one_nh(n, r, rta, rtnh, &argc, &argv);
+		if (parse_one_nh(n, r, rta, rtnh, &argc, &argv) < 0)
+			return -1;
 		rtnh = RTNH_NEXT(rtnh);
 	}
 
@@ -1318,8 +1319,8 @@ static int iproute_modify(int cmd, unsigned int flags, int argc, char **argv)
 		addattr_l(&req.n, sizeof(req), RTA_METRICS, RTA_DATA(mxrta), RTA_PAYLOAD(mxrta));
 	}
 
-	if (nhs_ok)
-		parse_nexthops(&req.n, &req.r, argc, argv);
+	if (nhs_ok && parse_nexthops(&req.n, &req.r, argc, argv) < 0)
+		return -1;
 
 	if (req.r.rtm_family == AF_UNSPEC)
 		req.r.rtm_family = AF_INET;
-- 
2.15.1