[ovs-dev] [PATCH 3/6] lldp: fix a buffer overflow when handling management address TLV
Aaron Conole
aconole at redhat.com
Tue Oct 27 12:54:33 UTC 2020
Hi Fabrizio,
Thanks for the work on this.
Fabrizio D'Angelo <fdangelo at redhat.com> writes:
> From: Vincent Bernat <vincent at bernat.im>
>
> Upstream commit:
> commit a8d8006c06d9ac16ebcf33295cbd625c0847ca9b
> Author: Vincent Bernat <vincent at bernat.im>
> Date: Sun, 4 Oct 2015 01:50:38 +0200
>
> lldp: fix a buffer overflow when handling management address TLV
>
> When a remote device was advertising a too large management address
> while still respecting TLV boundaries, lldpd would crash due to a buffer
> overflow. However, the buffer being a static one, this buffer overflow
> is not exploitable if hardening was not disabled. This bug exists since
> version 0.5.6.
>
> Co-authored-by: Fabrizio D'Angelo <fdangelo at redhat.com>
> Signed-off-by: Fabrizio D'Angelo <fdangelo at redhat.com>
> ---
> lib/lldp/lldp.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/lib/lldp/lldp.c b/lib/lldp/lldp.c
> index 593c5e1c34..aaa8ec842a 100644
> --- a/lib/lldp/lldp.c
> +++ b/lib/lldp/lldp.c
> @@ -530,6 +530,11 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int s,
> case LLDP_TLV_MGMT_ADDR:
> CHECK_TLV_SIZE(1, "Management address");
> addr_str_length = PEEK_UINT8;
> + if (addr_str_length > sizeof(addr_str_buffer)) {
> + VLOG_WARN(name "too large management address on %s",
> + hardware->h_ifname);
Two things about this change.
1. remove 'name', which seems to have been leftover from converting.
This causes a build error.
2. Align the hardware->h_ifname argument so that it will look like:
+ VLOG_WARN("too large management address on %s",
+ hardware->h_ifname);
> + goto malformed;
> + }
> CHECK_TLV_SIZE(1 + addr_str_length, "Management address");
> PEEK_BYTES(addr_str_buffer, addr_str_length);
> addr_length = addr_str_length - 1;
> @@ -554,7 +559,7 @@ lldp_decode(struct lldpd *cfg OVS_UNUSED, char *frame, int s,
> break;
>
> case LLDP_TLV_ORG:
> - CHECK_TLV_SIZE(4, "Organisational");
> + CHECK_TLV_SIZE(1 + (int)sizeof(orgid), "Organisational");
> PEEK_BYTES(orgid, sizeof orgid);
> tlv_subtype = PEEK_UINT8;
> if (memcmp(dot1, orgid, sizeof orgid) == 0) {
More information about the dev
mailing list