pathd: prevent stack buffer overflow in path_zebra_add_sr_policy()

[guoguojia2021]

path_zebra_add_sr_policy() iterates over all segment entries in a segment list and writes each SID label into zp.segment_list.labels[], which is a fixed-size array of MPLS_MAX_LABELS (16) elements on the stack. There is no bounds check on label_num before the write.

[greptile-apps]

Greptile Summary

This PR fixes a stack buffer overflow in path_zebra_add_sr_policy() by adding a bounds check before each write into the fixed-size zp.segment_list.labels[] array (size MPLS_MAX_LABELS = 16). Without the check, a segment list longer than 16 entries would silently overwrite stack memory.

• A >= MPLS_MAX_LABELS guard is inserted at the top of the RB_FOREACH loop; when the limit is hit, a zlog_warn is emitted and the loop breaks. The bounds check arithmetic is correct and the warning fires only once per call.
• After truncation, zebra_send_sr_policy() is still invoked with the partial label stack, which may install incorrect SR-TE forwarding state rather than failing safely.

Confidence Score: 3/5

The crash-causing overflow is prevented, but the truncation path still sends a partial label stack to Zebra, which can install forwarding state that misroutes or black-holes SR-TE traffic.

The bounds check is arithmetically correct and stops the overflow, which is the main goal. The residual concern is that after truncation the code continues to call zebra_send_sr_policy with an incomplete segment list, potentially pushing broken forwarding state to the data plane for any policy whose configured segment list exceeds 16 entries.

pathd/path_zebra.c — specifically the post-truncation flow that still invokes zebra_send_sr_policy.

Important Files Changed

Filename	Overview
pathd/path_zebra.c	Adds a bounds check before writing into the fixed-size labels[] array to prevent stack buffer overflow; however, on overflow it truncates and still sends the incomplete policy to Zebra, which can install incorrect SR-TE forwarding state.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[path_zebra_add_sr_policy called] --> B[Initialize zapi_sr_policy zp]
    B --> C{RB_FOREACH segment}
    C -->|next segment| D{label_num >= MPLS_MAX_LABELS?}
    D -->|Yes| E[zlog_warn: truncating]
    E --> F[break]
    D -->|No| G[Write label to array and increment label_num]
    G --> C
    C -->|done| H[policy status = GOING_UP]
    F --> H
    H --> I[zebra_send_sr_policy SET]
    I --> J{Was list truncated?}
    J -->|No - full list sent| K[Zebra installs correct policy]
    J -->|Yes - partial list sent| L[Zebra installs incomplete policy - potential misrouting]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[path_zebra_add_sr_policy called] --> B[Initialize zapi_sr_policy zp]
    B --> C{RB_FOREACH segment}
    C -->|next segment| D{label_num >= MPLS_MAX_LABELS?}
    D -->|Yes| E[zlog_warn: truncating]
    E --> F[break]
    D -->|No| G[Write label to array and increment label_num]
    G --> C
    C -->|done| H[policy status = GOING_UP]
    F --> H
    H --> I[zebra_send_sr_policy SET]
    I --> J{Was list truncated?}
    J -->|No - full list sent| K[Zebra installs correct policy]
    J -->|Yes - partial list sent| L[Zebra installs incomplete policy - potential misrouting]

Loading

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
pathd/path_zebra.c:188-198
**Truncated policy sent to Zebra with incomplete segment list**

When the segment list exceeds `MPLS_MAX_LABELS`, this code truncates the label stack and still calls `zebra_send_sr_policy()` with only the first 16 labels. Sending an incomplete segment list to the forwarding plane installs an SR-TE policy with a shorter stack than what was signaled, which will misroute or black-hole traffic at the first transit node that expects the full label stack. The original policy intent is not achievable with a partial label stack, so it may be safer to abort the install entirely (log the warning, skip the `zebra_send_sr_policy` call, and return) rather than silently forwarding with a truncated path.

_{Reviews (1): Last reviewed commit: "pathd: prevent stack buffer overflow in ..." | Re-trigger Greptile}

[greptile-apps]

Truncated policy sent to Zebra with incomplete segment list

When the segment list exceeds MPLS_MAX_LABELS, this code truncates the label stack and still calls zebra_send_sr_policy() with only the first 16 labels. Sending an incomplete segment list to the forwarding plane installs an SR-TE policy with a shorter stack than what was signaled, which will misroute or black-hole traffic at the first transit node that expects the full label stack. The original policy intent is not achievable with a partial label stack, so it may be safer to abort the install entirely (log the warning, skip the zebra_send_sr_policy call, and return) rather than silently forwarding with a truncated path.

Prompt To Fix With AI

This is a comment left during a code review.
Path: pathd/path_zebra.c
Line: 188-198

Comment:
**Truncated policy sent to Zebra with incomplete segment list**

When the segment list exceeds `MPLS_MAX_LABELS`, this code truncates the label stack and still calls `zebra_send_sr_policy()` with only the first 16 labels. Sending an incomplete segment list to the forwarding plane installs an SR-TE policy with a shorter stack than what was signaled, which will misroute or black-hole traffic at the first transit node that expects the full label stack. The original policy intent is not achievable with a partial label stack, so it may be safer to abort the install entirely (log the warning, skip the `zebra_send_sr_policy` call, and return) rather than silently forwarding with a truncated path.

How can I resolve this? If you propose a fix, please make it concise.

[riw777]

looks valid?

[riw777]

the one ai comment looks valid ?