From a78ea73f827928c1e19cfa5f76567b253c6e1213 Mon Sep 17 00:00:00 2001 From: renato Date: Sat, 16 Jul 2016 19:20:16 +0000 Subject: [PATCH] Fix parsing of mal-formed optional TLVs/Sub-TLVs. We must detect if a TLV's length extends beyond the end of the containing message. And, if so, send a fatal "Bad TLV Length" notification message. Found with the Mu Dynamics Mu-8000 protocol fuzzer. --- usr.sbin/ldpd/hello.c | 6 ++- usr.sbin/ldpd/init.c | 79 ++++++++++++++++-------------------- usr.sbin/ldpd/labelmapping.c | 17 ++++++-- usr.sbin/ldpd/notification.c | 14 ++++--- 4 files changed, 61 insertions(+), 55 deletions(-) diff --git a/usr.sbin/ldpd/hello.c b/usr.sbin/ldpd/hello.c index 63c93b7b115..f6d6245b3bd 100644 --- a/usr.sbin/ldpd/hello.c +++ b/usr.sbin/ldpd/hello.c @@ -1,4 +1,4 @@ -/* $OpenBSD: hello.c,v 1.56 2016/07/01 23:36:38 renato Exp $ */ +/* $OpenBSD: hello.c,v 1.57 2016/07/16 19:20:16 renato Exp $ */ /* * Copyright (c) 2013, 2016 Renato Westphal @@ -503,10 +503,12 @@ tlv_decode_opt_hello_prms(char *buf, uint16_t len, int *tlvs_rcvd, int af, */ while (len >= sizeof(tlv)) { memcpy(&tlv, buf, TLV_HDR_SIZE); + tlv_len = ntohs(tlv.length); + if (tlv_len + TLV_HDR_SIZE > len) + return (-1); buf += TLV_HDR_SIZE; len -= TLV_HDR_SIZE; total += TLV_HDR_SIZE; - tlv_len = ntohs(tlv.length); switch (ntohs(tlv.type)) { case TLV_TYPE_IPV4TRANSADDR: diff --git a/usr.sbin/ldpd/init.c b/usr.sbin/ldpd/init.c index 56cc14dcda0..f3bdf7b0648 100644 --- a/usr.sbin/ldpd/init.c +++ b/usr.sbin/ldpd/init.c @@ -1,4 +1,4 @@ -/* $OpenBSD: init.c,v 1.32 2016/07/01 23:36:38 renato Exp $ */ +/* $OpenBSD: init.c,v 1.33 2016/07/16 19:20:16 renato Exp $ */ /* * Copyright (c) 2009 Michele Marchetto @@ -25,7 +25,6 @@ #include "log.h" static int gen_init_prms_tlv(struct ibuf *, struct nbr *); -static int tlv_decode_opt_init_prms(char *, uint16_t); void send_init(struct nbr *nbr) @@ -59,7 +58,6 @@ recv_init(struct nbr *nbr, char *buf, uint16_t len) struct ldp_msg msg; struct sess_prms_tlv sess; uint16_t max_pdu_len; - int r; log_debug("%s: lsr-id %s", __func__, inet_ntoa(nbr->id)); @@ -93,11 +91,41 @@ recv_init(struct nbr *nbr, char *buf, uint16_t len) buf += SESS_PRMS_SIZE; len -= SESS_PRMS_SIZE; - /* just ignore all optional TLVs for now */ - r = tlv_decode_opt_init_prms(buf, len); - if (r == -1 || r != len) { - session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); - return (-1); + /* Optional Parameters */ + while (len > 0) { + struct tlv tlv; + uint16_t tlv_len; + + if (len < sizeof(tlv)) { + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); + return (-1); + } + + memcpy(&tlv, buf, TLV_HDR_SIZE); + tlv_len = ntohs(tlv.length); + if (tlv_len + TLV_HDR_SIZE > len) { + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); + return (-1); + } + buf += TLV_HDR_SIZE; + len -= TLV_HDR_SIZE; + + switch (ntohs(tlv.type)) { + case TLV_TYPE_ATMSESSIONPAR: + session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); + return (-1); + case TLV_TYPE_FRSESSION: + session_shutdown(nbr, S_BAD_TLV_VAL, msg.id, msg.type); + return (-1); + default: + if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) + send_notification_nbr(nbr, S_UNKNOWN_TLV, + msg.id, msg.type); + /* ignore unknown tlv */ + break; + } + buf += tlv_len; + len -= tlv_len; } nbr->keepalive = min(nbr_get_keepalive(nbr->af, nbr->id), @@ -136,38 +164,3 @@ gen_init_prms_tlv(struct ibuf *buf, struct nbr *nbr) return (ibuf_add(buf, &parms, SESS_PRMS_SIZE)); } - -static int -tlv_decode_opt_init_prms(char *buf, uint16_t len) -{ - struct tlv tlv; - uint16_t tlv_len; - int total = 0; - - while (len >= sizeof(tlv)) { - memcpy(&tlv, buf, TLV_HDR_SIZE); - buf += TLV_HDR_SIZE; - len -= TLV_HDR_SIZE; - total += TLV_HDR_SIZE; - tlv_len = ntohs(tlv.length); - - switch (ntohs(tlv.type)) { - case TLV_TYPE_ATMSESSIONPAR: - log_warnx("ATM session parameter present"); - return (-1); - case TLV_TYPE_FRSESSION: - log_warnx("FR session parameter present"); - return (-1); - default: - /* if unknown flag set, ignore TLV */ - if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) - return (-1); - break; - } - buf += tlv_len; - len -= tlv_len; - total += tlv_len; - } - - return (total); -} diff --git a/usr.sbin/ldpd/labelmapping.c b/usr.sbin/ldpd/labelmapping.c index a1ec305f0ca..f8261e6d6d8 100644 --- a/usr.sbin/ldpd/labelmapping.c +++ b/usr.sbin/ldpd/labelmapping.c @@ -1,4 +1,4 @@ -/* $OpenBSD: labelmapping.c,v 1.57 2016/07/15 17:09:25 renato Exp $ */ +/* $OpenBSD: labelmapping.c,v 1.58 2016/07/16 19:20:16 renato Exp $ */ /* * Copyright (c) 2014, 2015 Renato Westphal @@ -246,9 +246,13 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type) } memcpy(&tlv, buf, TLV_HDR_SIZE); + tlv_len = ntohs(tlv.length); + if (tlv_len + TLV_HDR_SIZE > len) { + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); + goto err; + } buf += TLV_HDR_SIZE; len -= TLV_HDR_SIZE; - tlv_len = ntohs(tlv.length); switch (ntohs(tlv.type)) { case TLV_TYPE_LABELREQUEST: @@ -334,10 +338,9 @@ recv_labelmessage(struct nbr *nbr, char *buf, uint16_t len, uint16_t type) } break; default: - if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) { + if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) send_notification_nbr(nbr, S_UNKNOWN_TLV, msg.id, msg.type); - } /* ignore unknown tlv */ break; } @@ -708,6 +711,12 @@ tlv_decode_fec_elm(struct nbr *nbr, struct ldp_msg *msg, char *buf, } memcpy(&stlv, buf + off, sizeof(stlv)); + if (stlv.length > pw_len) { + session_shutdown(nbr, S_BAD_TLV_LEN, msg->id, + msg->type); + return (-1); + } + switch (stlv.type) { case SUBTLV_IFMTU: if (stlv.length != FEC_SUBTLV_IFMTU_SIZE) { diff --git a/usr.sbin/ldpd/notification.c b/usr.sbin/ldpd/notification.c index 73788da9133..489001289a1 100644 --- a/usr.sbin/ldpd/notification.c +++ b/usr.sbin/ldpd/notification.c @@ -1,4 +1,4 @@ -/* $OpenBSD: notification.c,v 1.39 2016/07/01 23:36:38 renato Exp $ */ +/* $OpenBSD: notification.c,v 1.40 2016/07/16 19:20:16 renato Exp $ */ /* * Copyright (c) 2009 Michele Marchetto @@ -128,15 +128,18 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len) uint16_t tlv_len; if (len < sizeof(tlv)) { - session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, - msg.type); + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); return (-1); } memcpy(&tlv, buf, TLV_HDR_SIZE); + tlv_len = ntohs(tlv.length); + if (tlv_len + TLV_HDR_SIZE > len) { + session_shutdown(nbr, S_BAD_TLV_LEN, msg.id, msg.type); + return (-1); + } buf += TLV_HDR_SIZE; len -= TLV_HDR_SIZE; - tlv_len = ntohs(tlv.length); switch (ntohs(tlv.type)) { case TLV_TYPE_EXTSTATUS: @@ -167,10 +170,9 @@ recv_notification(struct nbr *nbr, char *buf, uint16_t len) nm.flags |= F_NOTIF_FEC; break; default: - if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) { + if (!(ntohs(tlv.type) & UNKNOWN_FLAG)) send_notification_nbr(nbr, S_UNKNOWN_TLV, msg.id, msg.type); - } /* ignore unknown tlv */ break; } -- 2.20.1