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.

diff --git a/usr.sbin/ldpd/hello.c b/usr.sbin/ldpd/hello.c
index 63c93b7..f6d6245 100644 (file)
--- 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 <renato@openbsd.org>
@@ -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 56cc14d..f3bdf7b 100644 (file)
--- 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 <michele@openbsd.org>
@@ -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 a1ec305..f8261e6 100644 (file)
--- 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 <renato@openbsd.org>
@@ -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 73788da..4890012 100644 (file)
--- 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 <michele@openbsd.org>
@@ -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;
                }