Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
bgpd: refine the update attr parse errors
To:
tech@openbsd.org
Date:
Thu, 19 Feb 2026 11:15:48 +0100

Download raw body.

Thread
  • Claudio Jeker:

    bgpd: refine the update attr parse errors

This diff is refining the error split I did in the previous commit to
rde.c by introducing one more common failure case with extra debug info.
I also adjusted the messages a bit, to be more self explanatory. I'm happy
to adjust those texts further since it is important to have messages that
operators understand without looking at the code.

I added a new message for the case where the attribute length causes an
overflow of the attribute buffer. This is when the ibuf_truncate() fails
since the alen is larger then the available data.
In many cases it is possible to extract the flags, types and even length
fields but the length value is for whatever reason garbage.

This diff includes a bug fix in the OTC parser for ROLE_PEER. At that
stage the parser should not consume the data in attrbuf and so it needs to
use an extra temporary buffer to extract the OTC value. This is similar to
the case in ATTR_AS4_AGGREGATOR where the same trick is used.
This is the second hunk, and I will commit this separately.

-- 
:wq Claudio

Index: rde.c
===================================================================
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
diff -u -p -r1.686 rde.c
--- rde.c	18 Feb 2026 15:54:06 -0000	1.686
+++ rde.c	19 Feb 2026 09:28:27 -0000
@@ -2056,7 +2056,7 @@ rde_attr_parse(struct ibuf *buf, struct 
 	}
 
 	if (ibuf_truncate(&attrbuf, alen) == -1)
-		goto bad_ibuf;
+		goto bad_size;
 	/* consume the attribute in buf before moving forward */
 	if (ibuf_skip(buf, hlen + alen) == -1)
 		goto bad_ibuf;
@@ -2375,7 +2375,8 @@ rde_attr_parse(struct ibuf *buf, struct 
 			a->flags |= F_ATTR_OTC_LEAK;
 			break;
 		case ROLE_PEER:
-			if (ibuf_get_n32(&attrbuf, &tmp32) == -1)
+			ibuf_from_ibuf(&tmpbuf, &attrbuf);
+			if (ibuf_get_n32(&tmpbuf, &tmp32) == -1)
 				goto bad_len;
 			if (tmp32 != peer->conf.remote_as)
 				a->flags |= F_ATTR_OTC_LEAK;
@@ -2407,11 +2408,18 @@ rde_attr_parse(struct ibuf *buf, struct 
 	rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRFLAGS, &attrbuf);
 	return (-1);
  bad_list:
-	log_peer_warnx(&peer->conf, "bad path, list error for type %d", type);
+	log_peer_warnx(&peer->conf, "bad update attributes, "
+	    "list error for attribute #%d", type);
 	rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL);
 	return (-1);
  bad_ibuf:
-	log_peer_warn(&peer->conf, "bad path, header parse error");
+	log_peer_warn(&peer->conf, "bad update attributes, "
+	    "message parse error");
+	rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL);
+	return (-1);
+ bad_size:
+	log_peer_warn(&peer->conf, "bad update attributes, "
+	    "attribute #%d [%x] with size %zu overflowed", type, flags, alen);
 	rde_update_err(peer, ERR_UPDATE, ERR_UPD_ATTRLIST, NULL);
 	return (-1);
 }