From: Vincent Bernat <vincent@bernat.ch>
Date: Sun, 6 Dec 2020 13:21:04 +0000 (+0100)
Subject: lib: fix LLDP-MED location parsing in liblldpctl
X-Git-Tag: 1.0.8~6^2
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fpull%2F422%2Fhead;p=thirdparty%2Flldpd.git

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
---

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 00000000..7e809516
Binary files /dev/null and b/tests/integration/data/med-loc-malformed.pcap differ
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
+        }