[PATCH] Wrong calculation in as_path_getlen, and check_aspa improvement suggestion
Alarig Le Lay
alarig at swordarmor.fr
Sun Aug 31 18:04:12 CEST 2025
Hello,
We (Evann and I) found a bug related to as_path_getlen() when used by
aspa_check(). When a route contains an AS_SET segment type, the length
returned by as_path_getlen() is incorrect. The function assumes that the
length of an AS_PATH_SET is a single AS (1), while in reality an
AS_PATH_SET is an unordered set of ASN (as described here
https://www.rfc-editor.org/rfc/rfc4271#section-9.2.2.1).
See the following update:
2025-08-31 15:35:15.134 <INFO> Checking prefix 76.165.0.0/16 (path 208627 29075 174 32440 {2055 10349 22985 23207 23294 23366 26002 26303 26333 30564 54529 396992 401290}) IN from bgp_alarig_ipv4
Using gdb we can inspect the path object:
(gdb) p path->length
$101 = 72
(gdb) x /72xb path->data
0x555555725e54: 0x02 0x04 0x00 0x03 0x2e 0xf3 0x00 0x00
0x555555725e5c: 0x71 0x93 0x00 0x00 0x00 0xae 0x00 0x00
0x555555725e64: 0x7e 0xb8 0x01 0x0d 0x00 0x00 0x08 0x07
0x555555725e6c: 0x00 0x00 0x28 0x6d 0x00 0x00 0x59 0xc9
0x555555725e74: 0x00 0x00 0x5a 0xa7 0x00 0x00 0x5a 0xfe
0x555555725e7c: 0x00 0x00 0x5b 0x46 0x00 0x00 0x65 0x92
0x555555725e84: 0x00 0x00 0x66 0xbf 0x00 0x00 0x66 0xdd
0x555555725e8c: 0x00 0x00 0x77 0x64 0x00 0x00 0xd5 0x01
0x555555725e94: 0x00 0x06 0x0e 0xc0 0x00 0x06 0x1f 0x8a
In this example, we have a route with an AS_PATH that contain:
- an AS_PATH_SEQUENCE (0x02) with a length of 4 (0x04): (208627
29075 174 32440);
- an AS_PATH_SET (0x01) with a length of 13 (0x0d): {2055 10349
22985 23207 23294 23366 26002 26303 26333 30564 54529 396992
401290}.
The total length of this update is then 17, but if we dump the function
result, we can see that the actual computed length is 5 (4 + 1 for the
AS_PATH_SET).
(gdb) p len
$103 = 5
This leads to a too small memory allocation, when normalizing the AS
Path in aspa_check():
/* Normalize the AS Path: drop stuffings */
u32 *asns = alloca(sizeof(u32) * len);
Causing a SEGFAULT during the as path walk. Since as_path_walk()
considers each element of the AS_PATH_SET as a step. In the while
(as_path_walk(path, &ppos, &asns[nsz])), the asns object should have a
size of 17 and not 5 resulting in overwriting memory and finally
triggering a SEGFAULT. (However we've seen this working when the AS_SET
is small, for example, it's working for the following route, but this is
mostly luck and could lead to weird behaviors):
Checking prefix 104.141.0.0/16 (path 208627 29075 174 32440 {400943}) IN from bgp_alarig_ipv4
Here is the gdb output showing this behaviour:
2025-08-31 15:35:15.134 <TRACE> bgp_alarig_ipv4: Got UPDATE
2025-08-31 15:35:15.134 <INFO> Checking prefix 76.165.0.0/16 (path 208627 29075 174 32440 {2055 10349 22985 23207 23294 23366 26002 26303 26333 30564 54529 396992 401290}) IN from bgp_alarig_ipv4
Thread 1 "bird" received signal SIGSEGV, Segmentation fault.
0x00005555555d68ac in as_path_walk (path=0x5555000066dd, pos=0x7fffffffd15c,
val=0x7fffffffd144) at nest/a-path.c:702
702 const u8 *q = p + path->length;
(gdb) p path->data
$1 = 0x5555000066e1 <error: Cannot access memory at address 0x5555000066e1>
And here is a dump of asns just before the segfault :
(gdb) p *asns at nsz+1
$57 = {208627, 29075, 174, 32440, 2055, 10349, 22985, 23207, 23294, 23366, 26002, 26303,
26333}
We propose to set the AS_PATH_SET length to the announced length in the
AS_PATH data instead of 1, see
0001-NEST-correct-as_path-len-calculation.patch.
Furthermore, as per
https://datatracker.ietf.org/doc/html/rfc9774#name-updates-to-the-requirements
(BGP speakers MUST use the "treat-as-withdraw" error handling behavior
per [RFC7606] upon reception of BGP UPDATE messages containing AS_SETs
or AS_CONFED_SETs in the AS_PATH or AS4_PATH [RFC6793]) and even if
https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-aspa-verification-22#name-as_path-verification
changes it to a SHOULD, another improvement we propose is to check for
AS_PATH_SET the same way it’s already done for AS_PATH_CONFED_SEQUENCE
and AS_PATH_CONFED_SET at the beginning of the aspa_check() (see
0002-NEST-return-ASPA_INVALID-for-path-containing-AS_SET.patch). The
proposed patch is only for ASPA, not for ROV, in order to avoid dropping
routes for too much people, and the patch only drop a few amounts of
routes (including a few routes dropped for invalid ROV) :
Routes: 1031692 imported, 212 filtered, 0 exported, 1031692 preferred
Don’t hesitate to discuss the patch if needed,
--
Alarig and Evann
-------------- next part --------------
>From 4f15decc09cd64768b92b7600eef4628ee0be7d6 Mon Sep 17 00:00:00 2001
From: Evann DREUMONT <53308142+LeGmask at users.noreply.github.com>
Date: Sun, 31 Aug 2025 17:52:11 +0200
Subject: [PATCH 1/2] NEST: correct as_path len calculation
AS_PATH_SET is wrongly statically set to 1, see https://www.rfc-editor.org/rfc/rfc4271#section-9.2.2.1
Co-Authored-By: Alarig <alarig at swordarmor.fr>
---
nest/a-path.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/nest/a-path.c b/nest/a-path.c
index aba2c86d..ba14065f 100644
--- a/nest/a-path.c
+++ b/nest/a-path.c
@@ -412,7 +412,7 @@ as_path_getlen(const struct adata *path)
switch (t)
{
- case AS_PATH_SET: n = 1; break;
+ case AS_PATH_SET: n = l; break;
case AS_PATH_SEQUENCE: n = l; break;
case AS_PATH_CONFED_SEQUENCE: n = 0; break;
case AS_PATH_CONFED_SET: n = 0; break;
--
2.51.0
-------------- next part --------------
>From 80129a73d4c9491e04fc9230b4579f5eae88f2ea Mon Sep 17 00:00:00 2001
From: Evann DREUMONT <53308142+LeGmask at users.noreply.github.com>
Date: Sun, 31 Aug 2025 17:53:31 +0200
Subject: [PATCH 2/2] NEST: return ASPA_INVALID for path containing AS_SET
See https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-aspa-verification#section-6
Co-Authored-By: Alarig <alarig at swordarmor.fr>
---
nest/a-path.c | 20 ++++++++++++++++++++
nest/attrs.h | 1 +
nest/rt-table.c | 5 +++++
3 files changed, 26 insertions(+)
diff --git a/nest/a-path.c b/nest/a-path.c
index ba14065f..ca3a40eb 100644
--- a/nest/a-path.c
+++ b/nest/a-path.c
@@ -177,6 +177,26 @@ as_path_contains_confed(const struct adata *path)
return 0;
}
+int
+as_path_contains_set(const struct adata *path)
+{
+ const byte *pos = path->data;
+ const byte *end = pos + path->length;
+
+ while (pos < end)
+ {
+ uint type = pos[0];
+ uint slen = 2 + BS * pos[1];
+
+ if (type == AS_PATH_SET)
+ return 1;
+
+ pos += slen;
+ }
+
+ return 0;
+}
+
struct adata *
as_path_strip_confed(struct linpool *pool, const struct adata *path)
{
diff --git a/nest/attrs.h b/nest/attrs.h
index 0475afa7..699b91df 100644
--- a/nest/attrs.h
+++ b/nest/attrs.h
@@ -36,6 +36,7 @@ int as_path_16to32(byte *dst, const byte *src, uint len);
int as_path_32to16(byte *dst, const byte *src, uint len);
int as_path_contains_as4(const struct adata *path);
int as_path_contains_confed(const struct adata *path);
+int as_path_contains_set(const struct adata *path);
struct adata *as_path_strip_confed(struct linpool *pool, const struct adata *op);
struct adata *as_path_prepend2(struct linpool *pool, const struct adata *op, int seq, u32 as);
struct adata *as_path_to_old(struct linpool *pool, const struct adata *path);
diff --git a/nest/rt-table.c b/nest/rt-table.c
index ee3f1188..ab7ce95f 100644
--- a/nest/rt-table.c
+++ b/nest/rt-table.c
@@ -362,6 +362,11 @@ enum aspa_result aspa_check(rtable *tab, const adata *path, bool force_upstream)
if (as_path_contains_confed(path))
return ASPA_INVALID;
+ /* No support for AS SET */
+ /* See https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-aspa-verification#section-6 */
+ if (as_path_contains_set(path))
+ return ASPA_INVALID;
+
/* Check path length */
uint len = as_path_getlen(path);
if (len == 0)
--
2.51.0
More information about the Bird-users
mailing list