summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorQian Chen <cq674350529@163.com>2022-12-20 09:36:08 -0500
committerIlya Maximets <i.maximets@ovn.org>2022-12-20 17:27:44 +0100
commitecaacb01a9d0210da3c228cce7289f0399630758 (patch)
tree668f5b4c50244e169d65afea3f03ff33c8677966
parentee002b35140e40c24e986965cf4e6561ccca1787 (diff)
downloadopenvswitch-ecaacb01a9d0210da3c228cce7289f0399630758.tar.gz
lldp: Fix bugs when parsing malformed AutoAttach.
The OVS LLDP implementation includes support for AutoAttach standard, which the 'upstream' lldpd project does not include. As part of adding this support, the message parsing for these TLVs did not include proper length checks for the LLDP_TLV_AA_ELEMENT_SUBTYPE and the LLDP_TLV_AA_ISID_VLAN_ASGNS_SUBTYPE elements. The result is that a message without a proper boundary will cause an overread of memory, and lead to undefined results, including crashes or other unidentified behavior. The fix is to introduce proper bounds checking for these elements. Introduce a unit test to ensure that we have some proper rejection in this code base in the future. Fixes: be53a5c447c3 ("auto-attach: Initial support for Auto-Attach standard") Signed-off-by: Qian Chen <cq674350529@163.com> Co-authored-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
-rw-r--r--lib/lldp/lldp.c2
-rw-r--r--tests/ofproto-dpif.at19
2 files changed, 21 insertions, 0 deletions
diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
index dfeb2a800..6fdcfef56 100644
--- a/lib/lldp/lldp.c
+++ b/lib/lldp/lldp.c
@@ -583,6 +583,7 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int s,
switch(tlv_subtype) {
case LLDP_TLV_AA_ELEMENT_SUBTYPE:
+ CHECK_TLV_SIZE(50, "ELEMENT");
PEEK_BYTES(&msg_auth_digest, sizeof msg_auth_digest);
aa_element_dword = PEEK_UINT32;
@@ -629,6 +630,7 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int s,
break;
case LLDP_TLV_AA_ISID_VLAN_ASGNS_SUBTYPE:
+ CHECK_TLV_SIZE(36, "ISID_VLAN_ASGNS");
PEEK_BYTES(&msg_auth_digest, sizeof msg_auth_digest);
/* Subtract off tlv type and length (2Bytes) + OUI (3B) +
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 73a5e8c3d..bc981f8fc 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -62,6 +62,25 @@ AT_CHECK([ovs-appctl coverage/read-counter rev_reconfigure], [0], [dnl
OVS_VSWITCHD_STOP
AT_CLEANUP
+AT_SETUP([ofproto-dpif - malformed lldp autoattach tlv])
+OVS_VSWITCHD_START()
+add_of_ports br0 1
+
+dnl Enable lldp
+AT_CHECK([ovs-vsctl set interface p1 lldp:enable=true])
+
+dnl Send a malformed lldp packet
+packet="0180c200000ef6b426aa5f0088cc020704f6b426aa5f000403057632060200780c"dnl
+"5044454144424545464445414442454546444541444245454644454144424545464445414"dnl
+"4424545464445414442454546444541444245454644454144424545464445414442454546"dnl
+"4445414442454546fe0500040d0c010000"
+AT_CHECK([ovs-appctl netdev-dummy/receive p1 "$packet"], [0], [stdout])
+
+OVS_WAIT_UNTIL([grep -q "ISID_VLAN_ASGNS TLV too short" ovs-vswitchd.log])
+
+OVS_VSWITCHD_STOP(["/|WARN|ISID_VLAN_ASGNS TLV too short received on/d"])
+AT_CLEANUP
+
AT_SETUP([ofproto-dpif - active-backup bonding (with primary)])
dnl Create br0 with members p1, p2 and p7, creating bond0 with p1 and