Skip to content

bgpd: Recent bugs for 10.4#21260

Merged
donaldsharp merged 9 commits intoFRRouting:stable/10.4from
opensourcerouting:fix/recent_security_fixes_10.4
Mar 19, 2026
Merged

bgpd: Recent bugs for 10.4#21260
donaldsharp merged 9 commits intoFRRouting:stable/10.4from
opensourcerouting:fix/recent_security_fixes_10.4

Conversation

@ton31337
Copy link
Copy Markdown
Member

No description provided.

…p received

There is no bounds check before the memcpy(). With Extended Message support
enabled, incoming OPEN messages can be up to 65535 bytes, so the total size
of unknown capability TLVs can far exceed 4096 bytes, overflowing the stack
buffer.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
We shouldn't allow processing AFI/SAFI received in route-refresh message if we
don't have this AFI/SAFI enabled for this peer.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
We didn't include confederation ASNs when counting hops, so let's count them in,
and return as early as possible the as-path as it is.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
The NOTIFY tears down the session asynchronously, but the function continues
parsing attacker-controlled bytes as ORF data before the session actually
closes.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
…ability

The GR function computes end 3 bytes too early (pnt vs data), causing
it to stop parsing 3 bytes before the real end.  For a carefully
crafted GR capability, the last AFI/SAFI entry (4 bytes) is silently
dropped.

Dynamic capability header is:

               +------------------------------+
               | Action (1 octet)             |
               +------------------------------+
               | Capability Code (1 octet)    |
               +------------------------------+
               | Capability Length (1 octet)  |
               +------------------------------+
               | Capability Value (variable)  |
               +------------------------------+

So we set wrongly the end of the packet (too early).

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
If "length" is 1 before the subtraction, "length -= 2" wraps to
65535 (unsigned).  The subsequent "sublength > length" check then
passes when it should fail, because length is now enormous.

The parser consumes bytes from adjacent attributes, corrupting the overall
attribute parse state. The STREAM_READABLE check at line 3184 prevents actual
buffer overflows, but attribute boundary violations can cause incorrect route
processing.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
When data + 1 == end (exactly 1 byte remaining, sufficient to read
the length byte), the function incorrectly rejects the capability.
Same issue at line 3498 for domain name length.

Also, hostname len can't be 0 (while domainname - can), so let's fix this too.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
We do this already with NEXT_HOP attribute, so let's do the same with
MP_REACH_NLRI attribute as well.

Reported-by: Jiahao Lei

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
Just to avoid memory leak.

Noticed randomly.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 19, 2026

Target branch is not in the allowed branches list.

@frrbot frrbot bot added the bgp label Mar 19, 2026
@donaldsharp donaldsharp merged commit 421ea91 into FRRouting:stable/10.4 Mar 19, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants