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 004f29346ed4463b187fbfedfefedae1a3b3a2a2..b8c2227def0b708687086ad4a0b1b8c850c08a59 100644 (file)
--- 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 e1b20fdf1c75d5c50b87677375a366de4d0c421d..595dba447395892d71b18d4f25dcb0e03de2ea1f 100644 (file)
--- 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 (file)
index 0000000..7e80951
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 4ecf7b57381076885b55dd739b69d2fac56aee33..5f3c0b1a4352026dc074a1fbe12531707714ac53 100644 (file)
--- 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
+        }