From 5c3479463a919193213213e2d8634c754c09aa51 Mon Sep 17 00:00:00 2001
From: Vincent Bernat <vincent@bernat.ch>
Date: Sun, 6 Dec 2020 14:21:04 +0100
Subject: [PATCH] lib: fix LLDP-MED location parsing in liblldpctl

Some bounds were not checked correctly when parsing LLDP-MED civic
location fields. This triggers out-of-bound reads (no write) in
lldpcli, ultimately leading to a crash.

Fix #420
---
 NEWS                                          |   5 ++++
 src/lib/atoms/med.c                           |   8 ++++--
 tests/integration/data/med-loc-malformed.pcap | Bin 0 -> 323 bytes
 tests/integration/test_pcap.py                |  25 ++++++++++++++++++
 4 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 tests/integration/data/med-loc-malformed.pcap

diff --git a/NEWS b/NEWS
index 004f2934..b8c2227d 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,8 @@
+lldpd (1.0.8)
+  * Fix:
+    + Out-of-bound read access when parsing LLDP-MED civic address in
+      liblldpctl for malformed fields.
+
 lldpd (1.0.7)
   * Fix:
     + Do not listen only to LLDP packets on Linux. When an interface
diff --git a/src/lib/atoms/med.c b/src/lib/atoms/med.c
index e1b20fdf..595dba44 100644
--- a/src/lib/atoms/med.c
+++ b/src/lib/atoms/med.c
@@ -540,6 +540,7 @@ _lldpctl_atom_get_str_med_location(lldpctl_atom_t *atom, lldpctl_key_t key)
 		return NULL;
 	case lldpctl_k_med_location_country:
 		if (m->location->format != LLDP_MED_LOCFORMAT_CIVIC) break;
+		if (m->location->data_len < 4) return NULL;
 		value = _lldpctl_alloc_in_atom(atom, 3);
 		if (!value) return NULL;
 		memcpy(value, m->location->data + 2, 2);
@@ -732,8 +733,11 @@ _lldpctl_atom_iter_med_caelements_list(lldpctl_atom_t *atom)
 {
 	struct _lldpctl_atom_med_caelements_list_t *plist =
 	    (struct _lldpctl_atom_med_caelements_list_t *)atom;
-	struct ca_iter *iter = _lldpctl_alloc_in_atom(atom, sizeof(struct ca_iter));
-	if (!iter) return NULL;
+	struct ca_iter *iter;
+	if (plist->parent->location->data_len < 4 ||
+	    *(uint8_t*)plist->parent->location->data < 3 ||
+	    !(iter = _lldpctl_alloc_in_atom(atom, sizeof(struct ca_iter))))
+		return NULL;
 	iter->data = (uint8_t*)plist->parent->location->data + 4;
 	iter->data_len = *(uint8_t*)plist->parent->location->data - 3;
 	return (lldpctl_atom_iter_t*)iter;
diff --git a/tests/integration/data/med-loc-malformed.pcap b/tests/integration/data/med-loc-malformed.pcap
new file mode 100644
index 0000000000000000000000000000000000000000..7e809516e657945b5cccc269351c81eac1daf250
GIT binary patch
literal 323
zc-p&ic+)~A1{MYcU}0bclG{G*i(up7WsnAPK$x-N5Ca1r*MWO;xk@_DFtIa2m@Mqf
zc3{B9#31mGgF$FF6G-<zE+C7UiOJE;l!1@sKM?$z%pk3Jgrk?Cj{z9~6*Dm~FoSGm
zU}a!nV_;y%D^5+&!0?X^=qScx3|yQkscETs#hJ;)94x6N83sHG&iN^+dHK1S$@zH-
cE-*=jkksN5h2YZ6lGK0fKp!$Pr~{n{0H$LyLI3~&

literal 0
Hc-jL100001

diff --git a/tests/integration/test_pcap.py b/tests/integration/test_pcap.py
index 4ecf7b57..5f3c0b1a 100644
--- a/tests/integration/test_pcap.py
+++ b/tests/integration/test_pcap.py
@@ -87,3 +87,28 @@ def test_8023bt(lldpd1, lldpcli, namespaces):
              'is not electrically isolated'),
             'lldp.eth0.port.power.max-power': '51000'
         }
+
+@pytest.mark.skipif("'LLDP-MED' not in config.lldpd.features",
+                    readon="LLDP-MED not supported")
+def test_med_loc_malformed(lldpd1, lldpcli, namespaces):
+    with namespaces(2):
+        pytest.helpers.send_pcap('data/med-loc-malformed.pcap', 'eth1')
+    with namespaces(1):
+        out = lldpcli("-f", "keyvalue", "show", "neighbors", "details")
+        for k in list(out.keys()):
+            if not k.startswith("lldp.eth0.lldp-med."):
+                del out[k]
+        assert out == {
+            'lldp.eth0.lldp-med.device-type': 'Communication Device Endpoint (Class III)',
+            'lldp.eth0.lldp-med.Capabilities.available': 'yes',
+            'lldp.eth0.lldp-med.Policy.available': 'yes',
+            'lldp.eth0.lldp-med.Location.available': 'yes',
+            'lldp.eth0.lldp-med.Inventory.available': 'yes',
+            'lldp.eth0.lldp-med.policy.apptype': 'Voice',
+            'lldp.eth0.lldp-med.policy.defined': 'yes',
+            'lldp.eth0.lldp-med.policy.priority': 'Best effort',
+            'lldp.eth0.lldp-med.policy.pcp': '0',
+            'lldp.eth0.lldp-med.policy.dscp': '0',
+            'lldp.eth0.lldp-med.Civic address.country': 'F5'
+            # Truncated
+        }
-- 
2.39.5