bgpd: fix off-by-one error in FlowSpec operator array bounds check

[Jafaral]

Change loop > BGP_PBR_MATCH_VAL_MAX to loop >= BGP_PBR_MATCH_VAL_MAX in bgp_flowspec_op_decode() and bgp_flowspec_bitmask_decode() to prevent writing one element past the end of the mval[] array when more than 5 chained operators are present in a FlowSpec component.

[greptile-apps]

Greptile Summary

This PR fixes an off-by-one buffer overflow in bgp_flowspec_op_decode() and bgp_flowspec_bitmask_decode() in bgpd/bgp_flowspec_util.c. The mval[] arrays have exactly BGP_PBR_MATCH_VAL_MAX (5) elements (valid indices 0–4), but the old loop > BGP_PBR_MATCH_VAL_MAX guard only triggered at loop == 6, allowing a write to mval[5] — one element past the end — when a FlowSpec component carried 6 or more chained operators. The fix tightens the check to loop >= BGP_PBR_MATCH_VAL_MAX, which correctly blocks at loop == 5.

Key changes:

• bgp_flowspec_op_decode(): condition changed from loop > BGP_PBR_MATCH_VAL_MAX to loop >= BGP_PBR_MATCH_VAL_MAX, and a missing return offset; is added — previously the function set *error = -2 but did not return, letting the loop body execute again (the write was only incidentally prevented by a secondary if (*error == -2) break; guard inside BGP_FLOWSPEC_CONVERT_TO_NON_OPAQUE).
• bgp_flowspec_bitmask_decode(): same condition tightened; a return offset; was already present here, so this is a pure fencepost fix.

Both changes are minimal, correct, and consistent with how the arrays are declared (struct bgp_pbr_match_val field[BGP_PBR_MATCH_VAL_MAX] throughout bgp_pbr.h).

Confidence Score: 5/5

• This PR is safe to merge — it is a focused, well-scoped security/correctness fix with no regressions introduced.
• The change is minimal (two condition tweaks and one missing return statement), directly addresses a real heap/stack buffer-overflow reachable from malformed BGP FlowSpec NLRI data, and matches the declared array sizes (BGP_PBR_MATCH_VAL_MAX == 5). No new logic paths are introduced and the error contract (*error = -2, early return) was already established in bgp_flowspec_bitmask_decode.
• No files require special attention beyond the reviewed fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["Enter decode loop — loop = 0"] --> B{"loop >= BGP_PBR_MATCH_VAL_MAX ?"}
    B -- Yes --> C["*error = -2\nreturn offset — array full"]
    B -- No --> D{"offset >= max_len ?"}
    D -- Yes --> E["*error = -1\nbreak — truncated input"]
    D -- No --> F["Parse operator byte\ncompute value_size"]
    F --> G{"BGP_FLOWSPEC_CONVERT\n_TO_NON_OPAQUE ?"}
    G -- Yes --> H["Write to mval, mval++"]
    G -- No --> I["String / validate path"]
    H --> J["offset += value_size\nloop++"]
    I --> J
    J --> K{"op[0] == 0 AND\noffset < max_len-1 ?"}
    K -- "Yes, more operators" --> B
    K -- "No, end-of-list" --> L["*error = loop\nreturn offset"]

Loading

_{Last reviewed commit: 0e6882b}

[Jafaral]

@greptile review

[Jafaral]

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

[mergify]

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

• #21058 bgpd: fix off-by-one error in FlowSpec operator array bounds check (backport #21054) has been created for branch stable/10.6
• #21059 bgpd: fix off-by-one error in FlowSpec operator array bounds check (backport #21054) has been created for branch stable/10.5
• #21060 bgpd: fix off-by-one error in FlowSpec operator array bounds check (backport #21054) has been created for branch stable/10.4
• #21061 bgpd: fix off-by-one error in FlowSpec operator array bounds check (backport #21054) has been created for branch stable/10.3
• #21062 bgpd: fix off-by-one error in FlowSpec operator array bounds check (backport #21054) has been created for branch stable/10.2
• #21063 bgpd: fix off-by-one error in FlowSpec operator array bounds check (backport #21054) has been created for branch stable/10.1
• #21064 bgpd: fix off-by-one error in FlowSpec operator array bounds check (backport #21054) has been created for branch stable/10.0

[ton31337]

LGTM