CVE-2017-13715: Improper Initialization (CWE-665)
value: The __skb_flow_dissect function in net/core/flow_dissector.c in the Linux kernel before 4.3 does not ensure that n_proto, ip_proto, and thoff are initialized, which allows remote attackers to cause a denial of service (system crash) or possibly execute arbitrary code via a single crafted MPLS packet.
CWE description: The product does not initialize or incorrectly initializes a resource, which might leave the resource in an unexpected state when it is accessed or used.
CVSS v3 base score: 9.8
Published: 2017-08-29T01:35Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: net/core/flow_dissector.c
Commit date: 2015-09-01 15:06:22 -0700
Commit message: flow_dissector: Jump to exit code in __skb_flow_dissect

Instead of returning immediately (on a parsing failure for instance) we
jump to cleanup code. This always sets protocol values in key_control
(even on a failure there is still valid information in the key_tags that
was set before the problem was hit).

Signed-off-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

Diff: diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 151b6e48b81f1d..22f3d768b45925 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -128,10 +128,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	struct flow_dissector_key_addrs *key_addrs;
 	struct flow_dissector_key_ports *key_ports;
 	struct flow_dissector_key_tags *key_tags;
 	struct flow_dissector_key_keyid *key_keyid;
 	u8 ip_proto = 0;
+	bool ret = false;
 
 	if (!data) {
 		data = skb->data;
 		proto = skb->protocol;
 		nhoff = skb_network_offset(skb);
@@ -169,11 +170,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		const struct iphdr *iph;
 		struct iphdr _iph;
 ip:
 		iph = __skb_header_pointer(skb, nhoff, sizeof(_iph), data, hlen, &_iph);
 		if (!iph || iph->ihl < 5)
-			return false;
+			goto out_bad;
 		nhoff += iph->ihl * 4;
 
 		ip_proto = iph->protocol;
 		if (ip_is_fragment(iph))
 			ip_proto = 0;
@@ -195,11 +196,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		__be32 flow_label;
 
 ipv6:
 		iph = __skb_header_pointer(skb, nhoff, sizeof(_iph), data, hlen, &_iph);
 		if (!iph)
-			return false;
+			goto out_bad;
 
 		ip_proto = iph->nexthdr;
 		nhoff += sizeof(struct ipv6hdr);
 
 		if (skb_flow_dissector_uses_key(flow_dissector,
@@ -232,11 +233,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		const struct vlan_hdr *vlan;
 		struct vlan_hdr _vlan;
 
 		vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan), data, hlen, &_vlan);
 		if (!vlan)
-			return false;
+			goto out_bad;
 
 		if (skb_flow_dissector_uses_key(flow_dissector,
 						FLOW_DISSECTOR_KEY_VLANID)) {
 			key_tags = skb_flow_dissector_target(flow_dissector,
 							     FLOW_DISSECTOR_KEY_VLANID,
@@ -254,52 +255,50 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			struct pppoe_hdr hdr;
 			__be16 proto;
 		} *hdr, _hdr;
 		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
 		if (!hdr)
-			return false;
+			goto out_bad;
 		proto = hdr->proto;
 		nhoff += PPPOE_SES_HLEN;
 		switch (proto) {
 		case htons(PPP_IP):
 			goto ip;
 		case htons(PPP_IPV6):
 			goto ipv6;
 		default:
-			return false;
+			goto out_bad;
 		}
 	}
 	case htons(ETH_P_TIPC): {
 		struct {
 			__be32 pre[3];
 			__be32 srcnode;
 		} *hdr, _hdr;
 		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
 		if (!hdr)
-			return false;
-		key_basic->n_proto = proto;
-		key_control->thoff = (u16)nhoff;
+			goto out_bad;
 
 		if (skb_flow_dissector_uses_key(flow_dissector,
 						FLOW_DISSECTOR_KEY_TIPC_ADDRS)) {
 			key_addrs = skb_flow_dissector_target(flow_dissector,
 							      FLOW_DISSECTOR_KEY_TIPC_ADDRS,
 							      target_container);
 			key_addrs->tipcaddrs.srcnode = hdr->srcnode;
 			key_control->addr_type = FLOW_DISSECTOR_KEY_TIPC_ADDRS;
 		}
-		return true;
+		goto out_good;
 	}
 
 	case htons(ETH_P_MPLS_UC):
 	case htons(ETH_P_MPLS_MC): {
 		struct mpls_label *hdr, _hdr[2];
 mpls:
 		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data,
 					   hlen, &_hdr);
 		if (!hdr)
-			return false;
+			goto out_bad;
 
 		if ((ntohl(hdr[0].entry) & MPLS_LS_LABEL_MASK) >>
 		     MPLS_LS_LABEL_SHIFT == MPLS_LABEL_ENTROPY) {
 			if (skb_flow_dissector_uses_key(flow_dissector,
 							FLOW_DISSECTOR_KEY_MPLS_ENTROPY)) {
@@ -308,25 +307,21 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 								      target_container);
 				key_keyid->keyid = hdr[1].entry &
 					htonl(MPLS_LS_LABEL_MASK);
 			}
 
-			key_basic->n_proto = proto;
-			key_basic->ip_proto = ip_proto;
-			key_control->thoff = (u16)nhoff;
-
-			return true;
+			goto out_good;
 		}
 
-		return true;
+		goto out_good;
 	}
 
 	case htons(ETH_P_FCOE):
 		key_control->thoff = (u16)(nhoff + FCOE_HEADER_LEN);
 		/* fall through */
 	default:
-		return false;
+		goto out_bad;
 	}
 
 ip_proto_again:
 	switch (ip_proto) {
 	case IPPROTO_GRE: {
@@ -335,11 +330,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			__be16 proto;
 		} *hdr, _hdr;
 
 		hdr = __skb_header_pointer(skb, nhoff, sizeof(_hdr), data, hlen, &_hdr);
 		if (!hdr)
-			return false;
+			goto out_bad;
 		/*
 		 * Only look inside GRE if version zero and no
 		 * routing
 		 */
 		if (hdr->flags & (GRE_VERSION | GRE_ROUTING))
@@ -355,11 +350,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 
 			keyid = __skb_header_pointer(skb, nhoff, sizeof(_keyid),
 						     data, hlen, &_keyid);
 
 			if (!keyid)
-				return false;
+				goto out_bad;
 
 			if (skb_flow_dissector_uses_key(flow_dissector,
 							FLOW_DISSECTOR_KEY_GRE_KEYID)) {
 				key_keyid = skb_flow_dissector_target(flow_dissector,
 								      FLOW_DISSECTOR_KEY_GRE_KEYID,
@@ -376,11 +371,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 
 			eth = __skb_header_pointer(skb, nhoff,
 						   sizeof(_eth),
 						   data, hlen, &_eth);
 			if (!eth)
-				return false;
+				goto out_bad;
 			proto = eth->h_proto;
 			nhoff += sizeof(*eth);
 		}
 		goto again;
 	}
@@ -393,11 +388,11 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 			break;
 
 		opthdr = __skb_header_pointer(skb, nhoff, sizeof(_opthdr),
 					      data, hlen, &_opthdr);
 		if (!opthdr)
-			return false;
+			goto out_bad;
 
 		ip_proto = opthdr[0];
 		nhoff += (opthdr[1] + 1) << 3;
 
 		goto ip_proto_again;
@@ -413,24 +408,28 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 		goto mpls;
 	default:
 		break;
 	}
 
-	key_basic->n_proto = proto;
-	key_basic->ip_proto = ip_proto;
-	key_control->thoff = (u16)nhoff;
-
 	if (skb_flow_dissector_uses_key(flow_dissector,
 					FLOW_DISSECTOR_KEY_PORTS)) {
 		key_ports = skb_flow_dissector_target(flow_dissector,
 						      FLOW_DISSECTOR_KEY_PORTS,
 						      target_container);
 		key_ports->ports = __skb_flow_get_ports(skb, nhoff, ip_proto,
 							data, hlen);
 	}
 
-	return true;
+out_good:
+	ret = true;
+
+out_bad:
+	key_basic->n_proto = proto;
+	key_basic->ip_proto = ip_proto;
+	key_control->thoff = (u16)nhoff;
+
+	return ret;
 }
 EXPORT_SYMBOL(__skb_flow_dissect);
 
 static u32 hashrnd __read_mostly;
 static __always_inline void __flow_hash_secret_init(void)

Vulnerable code: 			return false;
			return false;
			return false;
			return false;
			return false;
			return false;
		key_basic->n_proto = proto;
		key_control->thoff = (u16)nhoff;
		return true;
			return false;
			key_basic->n_proto = proto;
			key_basic->ip_proto = ip_proto;
			key_control->thoff = (u16)nhoff;

			return true;
		return true;
		return false;
			return false;
				return false;
				return false;
			return false;
	key_basic->n_proto = proto;
	key_basic->ip_proto = ip_proto;
	key_control->thoff = (u16)nhoff;

	return true;
Fixed code: 	bool ret = false;
			goto out_bad;
			goto out_bad;
			goto out_bad;
			goto out_bad;
			goto out_bad;
			goto out_bad;
		goto out_good;
			goto out_bad;
			goto out_good;
		goto out_good;
		goto out_bad;
			goto out_bad;
				goto out_bad;
				goto out_bad;
			goto out_bad;
out_good:
	ret = true;

out_bad:
	key_basic->n_proto = proto;
	key_basic->ip_proto = ip_proto;
	key_control->thoff = (u16)nhoff;

	return ret;