net: coap: empty payload with payload marker is malformed

From RFC 7252, section 3 "The absence of the Payload Marker denotes a zero-length payload. The presence of a marker followed by a zero-length payload MUST be processed as a message format error." Check empty payload when COAP_MARKER is found and add a test case to cover it Signed-off-by: Robert Chou <[email protected]>
olofj · Oct 25, 2017 · 7aa2231 · 7aa2231
1 parent c1881b2
commit 7aa2231
Show file tree

Hide file tree

Showing 2 changed files with 66 additions and 1 deletion.
diff --git a/subsys/net/lib/coap/coap.c b/subsys/net/lib/coap/coap.c
@@ -166,7 +166,8 @@ static int parse_option(const struct coap_packet *cpkt,
 
 	/* This indicates that options have ended */
 	if (opt == COAP_MARKER) {
-		return 0;
+		/* packet w/ marker but no payload is malformed */
+		return r > 0 ? 0 : -EINVAL;
 	}
 
 	delta = option_header_get_delta(opt);

diff --git a/tests/net/lib/coap/src/main.c b/tests/net/lib/coap/src/main.c
@@ -1490,6 +1490,68 @@ static int test_parse_malformed_opt_len_ext(void)
 	return result;
 }
 
+/* IPv6 + UDP frame (48 bytes) */
+static const unsigned char ipv6_malformed_marker[] = {
+	/* IPv6 header starts here */
+	0x60, 0x00, 0x00, 0x00, 0x00, 0x0E, 0x11, 0xFF,
+	0x20, 0x01, 0x0D, 0xB8, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02,
+	0x20, 0x01, 0x0D, 0xB8, 0x00, 0x00, 0x00, 0x00,
+	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
+	/* UDP */
+	0xd8, 0xb4, 0x16, 0x33, 0x00, 0x0E, 0x00, 0x00,
+	/* CoAP */
+};
+
+/* 1 option, No payload (with payload marker) */
+static int test_parse_malformed_marker(void)
+{
+	u8_t pdu[] = { 0x40, 0x01, 0, 0, 0x40, 0xFF};
+	struct net_pkt *pkt;
+	struct net_buf *frag;
+	struct coap_packet cpkt;
+	int result = TC_FAIL;
+	int r;
+
+	pkt = net_pkt_get_reserve(&coap_pkt_slab, 0, K_NO_WAIT);
+	if (!pkt) {
+		TC_PRINT("Could not get packet from pool\n");
+		goto done;
+	}
+
+	frag = net_buf_alloc(&coap_data_pool, K_NO_WAIT);
+	if (!frag) {
+		TC_PRINT("Could not get buffer from pool\n");
+		goto done;
+	}
+
+	net_pkt_frag_add(pkt, frag);
+
+	net_pkt_append_all(pkt, sizeof(ipv6_malformed_marker),
+			   (u8_t *)ipv6_malformed_marker, K_FOREVER);
+	net_pkt_append_all(pkt, sizeof(pdu), (u8_t *)pdu, K_FOREVER);
+
+	net_pkt_set_ip_hdr_len(pkt, NET_IPV6H_LEN);
+	net_pkt_set_ipv6_ext_len(pkt, 0);
+
+	memcpy(frag->data, pdu, sizeof(pdu));
+	frag->len = NET_IPV6UDPH_LEN + sizeof(pdu);
+
+	/* an empty payload packet with payload marker is malformed */
+	r = coap_packet_parse(&cpkt, pkt, NULL, 0);
+	if (r) {
+		result = TC_PASS;
+	}
+
+done:
+	net_pkt_unref(pkt);
+
+	TC_END_RESULT(result);
+
+	return result;
+}
+
+
 static const struct {
 	const char *name;
 	int (*func)(void);
@@ -1509,6 +1571,8 @@ static const struct {
 	{ "Parse malformed option ext", test_parse_malformed_opt_ext },
 	{ "Parse malformed option length ext",
 		test_parse_malformed_opt_len_ext },
+	{ "Parse malformed empty payload with marker",
+		test_parse_malformed_marker, },
 };
 
 int main(int argc, char *argv[])