Skip to content

bgpd: improve packet parsing for EVPN and ENCAP/VNC#21098

Merged
riw777 merged 1 commit intoFRRouting:masterfrom
mjstapp:fix_bgp_parse_evpn_vnc
Mar 18, 2026
Merged

bgpd: improve packet parsing for EVPN and ENCAP/VNC#21098
riw777 merged 1 commit intoFRRouting:masterfrom
mjstapp:fix_bgp_parse_evpn_vnc

Conversation

@mjstapp
Copy link
Copy Markdown
Contributor

@mjstapp mjstapp commented Mar 11, 2026

Improve packet validation for EVPN NLRIs and for ENCAP/VNC. Validate internal ip address fields against overall message length; impose stricter validation for VNC sub-tlvs in the rfapi code.

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR hardens packet parsing for EVPN (Types 2, 3, and 4) and VNC/RFAPI by adding cross-validation between the internally-declared IP address length field and the overall NLRI wire-length, and by adding stricter sub-TLV length guards in the rfapi RIB path.

Key changes:

  • bgpd/bgp_evpn.c — Type-2: A new psize vs. ipaddr_len consistency check is added after ipaddr_len has already been validated to be 0, 32, or 128, so integer division is exact and no fringe values can slip through.
  • bgpd/bgp_evpn.c — Type-3 and bgpd/bgp_evpn_mh.c — Type-4: Similar cross-validation checks are added, but they are placed before the explicit bit-length guard against IPV4_MAX_BITLEN/IPV6_MAX_BITLEN. Because psize is independently constrained to two legal values before these checks run, any non-byte-aligned ipaddr_len that passes via truncating integer division (e.g., 33/8 == 4) will still be rejected by the subsequent explicit bit-length comparison. There is no memory safety risk, but the ordering means the new guards are not independently sufficient on their own.
  • bgpd/rfapi/rfapi_rib.c: Two new guards prevent processing a BGP_VNC_SUBTLV_TYPE_RFPOPTION sub-TLV whose pEncap->length is less than 3 (which would allow a zero-byte allocation and a potentially out-of-bounds value[1] read) or whose declared option length (value[1]) is zero. This closes a previously identified vulnerability where XCALLOC(..., 0) could be called, the allocation would succeed, and a warning-and-shrink loop would produce a hop with a zeroed length.

Confidence Score: 4/5

  • This PR is safe to merge — all new validation checks are defensive guards that reject malformed input and do not introduce new memory safety issues.
  • The changes add strictly defensive input validation. The rfapi fix is clean and complete. For EVPN Types 3 and 4, the new size check uses integer division before ipaddr_len is validated against canonical values, meaning non-byte-aligned values can pass it — but they are caught by the subsequent explicit bit-length check, so no unsafe memory operation can occur. The Type-2 check is ordered correctly and is fully robust. Score docked one point because of the ordering inconsistency between Type-2 and Types 3/4, which leaves the new guards in Types 3/4 not independently sufficient.
  • No files require special attention; the logic is correct and no regressions are expected.

Important Files Changed

Filename Overview
bgpd/bgp_evpn.c Adds cross-validation of ipaddr_len against psize for Type-2 (after prior ipaddr_len validation — robust) and Type-3 (before ipaddr_len validation against valid bit-lengths — relies on subsequent check to reject fringe values). No memory safety issues introduced.
bgpd/bgp_evpn_mh.c Adds psize/ipaddr_len cross-validation for Type-4 routes. The outer psize check (lines 830–836) against the two legal sizes already constrains psize; the new check further ensures the internal ip-len field is consistent with that size. Safe due to subsequent explicit bit-length check.
bgpd/rfapi/rfapi_rib.c Adds two guards for BGP_VNC_SUBTLV_TYPE_RFPOPTION: rejects sub-TLVs shorter than 3 bytes (preventing out-of-bounds value[1] access and zero-byte allocations) and rejects a declared option length of zero. Allocation and memcpy are now always operating on at least 1 byte. Change is correct and complete.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Receive NLRI packet] --> B{Route Type?}

    B -->|Type-2| T2A[Validate psize is one of 33 36 37 40 49 52]
    T2A -->|invalid| FAIL1[return -1]
    T2A -->|valid| T2B[Read macaddr_len and ipaddr_len]
    T2B --> T2C{ipaddr_len is 0 or 32 or 128?}
    T2C -->|no| FAIL2[goto fail]
    T2C -->|yes| T2D{psize == 33 + ipaddr_len/8 OR 36 + ipaddr_len/8? NEW}
    T2D -->|no| FAIL3[goto fail]
    T2D -->|yes| T2E[Read IP and VNI labels then process route]

    B -->|Type-3| T3A{psize == 17 or 29?}
    T3A -->|no| FAIL4[return -1]
    T3A -->|yes| T3B[Read Eth Tag then ipaddr_len]
    T3B --> T3C{psize == 13 + ipaddr_len/8? NEW}
    T3C -->|no| FAIL5[return -1]
    T3C -->|yes| T3D{ipaddr_len == 32 or 128?}
    T3D -->|no| FAIL6[return -1]
    T3D -->|yes| T3E[Read IP then process route]

    B -->|Type-4| T4A{psize == 23 or 35?}
    T4A -->|no| FAIL7[return -1]
    T4A -->|yes| T4B[Read ESI then ipaddr_len]
    T4B --> T4C{psize == 19 + ipaddr_len/8? NEW}
    T4C -->|no| FAIL8[return -1]
    T4C -->|yes| T4D{ipaddr_len == 32 or 128?}
    T4D -->|no| FAIL9[return -1]
    T4D -->|yes| T4E[Read IP then process route]

    B -->|VNC RFPOPTION| V1{pEncap length less than 3? NEW}
    V1 -->|yes| VBREAK[skip sub-TLV]
    V1 -->|no| V2{pEncap value index 1 equals 0? NEW}
    V2 -->|yes| VBREAK
    V2 -->|no| V3[Allocate hop copy value bytes shrink length if mismatch]
Loading

Last reviewed commit: 7676cad

Improve packet validation for EVPN NLRIs and for ENCAP/VNC.

Signed-off-by: Mark Stapp <mjs@cisco.com>
@mjstapp mjstapp force-pushed the fix_bgp_parse_evpn_vnc branch from 4369990 to 7676cad Compare March 11, 2026 20:14
@Jafaral
Copy link
Copy Markdown
Member

Jafaral commented Mar 11, 2026

@Mergifyio backport stable/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2 stable/10.1 stable/10.0

@mergify
Copy link
Copy Markdown

mergify bot commented Mar 11, 2026

backport stable/10.6 stable/10.5 stable/10.4 stable/10.3 stable/10.2 stable/10.1 stable/10.0

✅ Backports have been created

Details

@Jafaral
Copy link
Copy Markdown
Member

Jafaral commented Mar 11, 2026

@greptile review

Copy link
Copy Markdown
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@riw777 riw777 merged commit 4825b5b into FRRouting:master Mar 18, 2026
20 checks passed
riw777 added a commit that referenced this pull request Mar 18, 2026
bgpd: improve packet parsing for EVPN and ENCAP/VNC (backport #21098)
riw777 added a commit that referenced this pull request Mar 18, 2026
bgpd: improve packet parsing for EVPN and ENCAP/VNC (backport #21098)
riw777 added a commit that referenced this pull request Mar 18, 2026
bgpd: improve packet parsing for EVPN and ENCAP/VNC (backport #21098)
riw777 added a commit that referenced this pull request Mar 18, 2026
bgpd: improve packet parsing for EVPN and ENCAP/VNC (backport #21098)
riw777 added a commit that referenced this pull request Mar 18, 2026
bgpd: improve packet parsing for EVPN and ENCAP/VNC (backport #21098)
riw777 added a commit that referenced this pull request Mar 18, 2026
bgpd: improve packet parsing for EVPN and ENCAP/VNC (backport #21098)
riw777 added a commit that referenced this pull request Mar 18, 2026
bgpd: improve packet parsing for EVPN and ENCAP/VNC (backport #21098)
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.

3 participants