pimd: fix heap OOB write in BSM fragmenter

[Jafaral]

Prevent pre-auth heap overflow when forwarding oversized BSMs with zero-RP group records by bounding fragment output and source RP copies, and reject rp_count/frag_rp_count mismatches at parse time.

[greptile-apps]

Greptile Summary

This PR hardens the BSM fragment forwarder (pim_bsm_frag_send) and parser (pim_bsm_parse_install_g2rp) against malformed or crafted Bootstrap messages that could trigger a pre-authentication heap out-of-bounds write. A new pim_bsm_frag_flush helper de-duplicates the send-and-reset logic, and several input-validation checks are inserted at parse and fragment time.

• Fragmenter bounds checks: group-header truncation, frag_rp_count × RP_LEN > remaining-bytes, and fragment-output overflow are now rejected with return false; the outer-loop flush threshold was corrected to GRP_LEN + RP_LEN to guarantee at least one RP fits after writing the group header.
• Parser mismatch rejection: two new guards in pim_bsm_parse_install_g2rp reject packets where rp_count == 0 but frag_rp_count != 0, and where frag_rp_count > rp_count, closing the ghost-RP-entry attack vector.
• Zero-RP group forwarding: groups carrying frag_rp_count == 0 in a fragment are now forwarded correctly instead of triggering the inner RP loop.

Confidence Score: 4/5

The security-critical fragmentation and parse paths are materially improved; no new OOB write vectors introduced, but a handful of edge paths (zero-RP group continuation, frag_rp_cnt invariant comment accuracy) deserve a second look before merge.

The fixes are well-structured and address the core heap-OOB write. The frag_rp_cnt == 0 guard is marked "Unreachable" in a comment but the comment overstates the invariant: after a flush inside the inner loop, this_pkt_rem is reset and the group header is re-written at lines 1067–1071, so rp_fit_cnt is recomputed from the fresh this_pkt_rem. The invariant does hold in practice, but only because the inner-loop MTU check at line 1056 exits early — the comment does not explain this secondary guard, making the safety reasoning harder to verify during future refactors. This is a documentation/maintenance concern rather than a present defect.

pimd/pim_bsm.c — specifically the interaction between the inner-loop flush at line 1044, the continuation group-header copy at lines 1067–1071, and the frag_rp_cnt == 0 guard comment at lines 1001–1005.

Important Files Changed

Filename	Overview
pimd/pim_bsm.c	Adds heap-OOB defences to the BSM fragmenter (bounds checks on group/RP counts, corrected flush threshold, frag_rp_count/rp_count mismatch rejection at parse time) and factors out flush logic into pim_bsm_frag_flush(); changes are targeted and correct but one edge interaction with the pak_pending flag in zero-RP groups warrants attention.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant pim_bsm_frag_send
    participant pim_bsm_frag_flush
    participant pim_bsm_send_intf

    Caller->>pim_bsm_frag_send: buf, len, pim_mtu
    pim_bsm_frag_send->>pim_bsm_frag_send: XCALLOC(pak_start, pim_mtu)

    loop For each group in buf
        pim_bsm_frag_send->>pim_bsm_frag_send: "bounds check parsed_len+GRP_LEN <= len"
        alt "this_pkt_rem < GRP_LEN+RP_LEN"
            pim_bsm_frag_send->>pim_bsm_frag_flush: flush current fragment
            pim_bsm_frag_flush->>pim_bsm_send_intf: send fragment
            pim_bsm_frag_flush-->>pim_bsm_frag_send: reset pkt, this_pkt_rem
        end
        pim_bsm_frag_send->>pim_bsm_frag_send: memcpy group header into fragment
        alt "frag_rp_count == 0"
            pim_bsm_frag_send->>pim_bsm_frag_send: "pak_pending=true, continue"
        else "frag_rp_count > 0"
            pim_bsm_frag_send->>pim_bsm_frag_send: "bounds frag_rp_count*RP_LEN <= remaining"
            loop "While this_rp_cnt < total_rp_cnt"
                pim_bsm_frag_send->>pim_bsm_frag_send: compute frag_rp_cnt
                pim_bsm_frag_send->>pim_bsm_frag_send: "guard frag_rp_cnt != 0"
                pim_bsm_frag_send->>pim_bsm_frag_send: "bounds parsed_len+copy <= len"
                pim_bsm_frag_send->>pim_bsm_frag_send: memcpy RPs into fragment
                alt fragment full or all RPs done
                    pim_bsm_frag_send->>pim_bsm_frag_flush: flush fragment
                    pim_bsm_frag_flush->>pim_bsm_send_intf: send fragment
                end
            end
        end
    end

    alt pak_pending
        pim_bsm_frag_send->>pim_bsm_send_intf: send final fragment
    end
    pim_bsm_frag_send->>pim_bsm_frag_send: XFREE(pak_start)
    pim_bsm_frag_send-->>Caller: true

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
pimd/pim_bsm.c:1001-1006
The "Unreachable" comment is incomplete. It explains only the outer-loop pre-flush path but ignores the continuation path: after `pim_bsm_frag_flush` inside the inner loop, the group header is re-written at lines 1067–1071 and `this_pkt_rem` is decremented by `PIM_BSM_GRP_LEN`. The zero-entry guard only stays unreachable because the MTU check at line 1056 exits early when `this_pkt_rem < GRP_LEN + RP_LEN` after the inner flush. Without documenting this second invariant, a future refactor that relaxes or removes the line-1056 check could silently reach this guard — which then `return false`s rather than truly being unreachable.

```suggestion
			/* Unreachable under correct invariants:
			 * (a) outer-loop pre-flush (line ~951) guarantees
			 *     this_pkt_rem >= GRP_LEN + RP_LEN before the
			 *     first group header write, so rp_fit_cnt >= 1;
			 * (b) after an inner-loop flush + continuation group
			 *     header copy (lines ~1067-1071), the MTU check
			 *     at line ~1056 has already enforced
			 *     this_pkt_rem >= GRP_LEN + RP_LEN, so
			 *     rp_fit_cnt >= 1 there too.
			 * Guard anyway so a future refactor that drops either
			 * check cannot silently spin forever.
			 */
```

_{Reviews (8): Last reviewed commit: "pimd: fix heap OOB write in BSM fragment..." | Re-trigger Greptile}

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@Mergifyio backport stable/10.6 stable/10.5

[mergify]

backport stable/10.6 stable/10.5

✅ Backports have been created

Details

• #22225 pimd: fix heap OOB write in BSM fragmenter (backport #22222) has been created for branch stable/10.6
• #22226 pimd: fix heap OOB write in BSM fragmenter (backport #22222) has been created for branch stable/10.5