ospf6d: reject undersized LSAs in ospf6_lsupdate_recv

[uwezkhan]

Summary

ospf6_lsupdate_recv walks the LSAs in a received Link State Update by stepping p forward by ospf6_lsa_size(), which is ntohs(header->length) straight off the wire. The walk only checks that each LSA fits inside the message, not that its claimed length covers a full LSA header, so a neighbor in Exchange, Loading or Full can send an LS Update whose LSA length field is below 20. ospf6_receive_lsa then passes it to ospf6_lsa_create, which mallocs that many bytes and copies them, and ospf6_lsa_checksum_valid reads the length field at offset 18 of that short copy and runs fletcher_checksum over ntohs(length) - 2 bytes, which underflows to about 65534 for a length of 0 or 1.

Before, any LSA length that fit the remaining message was processed, so a 1-byte LSA produced a 1-byte allocation that was then read well past its end. After, the walk also requires ospf6_lsa_size() to be at least the size of the LSA header before the LSA is handed on, the same fixed-size bound the database-description, request and ack walks in this file already rely on. Keeping the bound in the walk means one malformed length stops parsing instead of every downstream consumer carrying its own guard; the tradeoff is that a truncated LSA ends processing of the rest of the update, matching the existing too-long check.

Related Issue

N/A

Components

ospf6d

[greptile-apps]

Greptile Summary

This PR adds a ospf6_lsa_size() >= sizeof(struct ospf6_lsa_header) guard to the LSA-walk loop in ospf6_lsupdate_recv, preventing undersized LSA lengths from being passed to ospf6_lsa_create and triggering a heap over-read or integer underflow in the checksum path.

• The guard is logically correct for the stated goal, but the first loop condition remains p < OSPF6_MESSAGE_END(oh), which allows ospf6_lsa_size() to be invoked when fewer than 20 bytes remain — the length field sits at offset 18 in the header, so this is itself an out-of-bounds read on a crafted short tail.
• The companion debug-print function ospf6_lsupdate_print retains the original unguarded pattern entirely and is not updated.

Confidence Score: 3/5

The receive loop still calls ospf6_lsa_size() without first confirming that a full 20-byte header is readable, leaving a partial out-of-bounds read reachable from the same crafted packet the PR aims to block.

The stated fix (preventing short LSA lengths from reaching ospf6_lsa_create) is valid, but the guard is incomplete: ospf6_lsa_size() reads header->length at byte offset 18, yet the loop's first condition only requires p < OSPF6_MESSAGE_END, meaning a packet whose tail is 1–19 bytes still triggers an out-of-bounds read before the new check can fire. Additionally ospf6_lsupdate_print is left entirely unguarded and processes the same input under debug mode.

ospf6d/ospf6_message.c — both the receive loop (line 1650) and the debug print loop (line 218) need attention

Important Files Changed

Filename	Overview
ospf6d/ospf6_message.c	Adds minimum-header-size guard to ospf6_lsupdate_recv LSA walk, but the first loop condition still allows ospf6_lsa_size() to be called before confirming a full header is present; ospf6_lsupdate_print retains the original unsafe pattern entirely.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Receive LS Update packet] --> B[Enter ospf6_lsupdate_recv loop]
    B --> C{p < OSPF6_MESSAGE_END?}
    C -- No --> Z[Stop: end of message]
    C -- Yes --> D{ospf6_lsa_size >= sizeof header?\n⚠ reads offset 18 — needs ≥20 bytes}
    D -- No: size too small --> Z2[Stop: malformed LSA, break loop]
    D -- Yes --> E{p + ospf6_lsa_size <= OSPF6_MESSAGE_END?}
    E -- No: LSA overruns packet --> Z3[Stop: truncated LSA, break loop]
    E -- Yes --> F[ospf6_receive_lsa → ospf6_lsa_create → process LSA]
    F --> G[p += ospf6_lsa_size]
    G --> C

Loading

Comments Outside Diff (1)

1. ospf6d/ospf6_message.c, line 218-226 (link)

   ospf6_lsupdate_print still carries the pre-fix unsafe pattern

   The debug print loop here has the same p < OSPF6_MESSAGE_END(oh) && p + ospf6_lsa_size(p) <= ... structure that was left unpatched. A crafted LSU whose tail contains fewer than 20 bytes will cause ospf6_lsa_size() to read header->length at offset 18 out of bounds. While this path is gated on debug flags, the same well-formed packet that exercises ospf6_lsupdate_recv also goes through ospf6_lsupdate_print when debug logging is enabled, so the guard added to the receive loop should be applied here too.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: ospf6d/ospf6_message.c
   Line: 218-226
   
   Comment:
   **`ospf6_lsupdate_print` still carries the pre-fix unsafe pattern**
   
   The debug print loop here has the same `p < OSPF6_MESSAGE_END(oh) && p + ospf6_lsa_size(p) <= ...` structure that was left unpatched. A crafted LSU whose tail contains fewer than 20 bytes will cause `ospf6_lsa_size()` to read `header->length` at offset 18 out of bounds. While this path is gated on debug flags, the same well-formed packet that exercises `ospf6_lsupdate_recv` also goes through `ospf6_lsupdate_print` when debug logging is enabled, so the guard added to the receive loop should be applied here too.
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

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

---

### Issue 1 of 2
ospf6d/ospf6_message.c:1650-1655
**Missing full-header guard before `ospf6_lsa_size()` call**

`ospf6_lsa_size()` reads `header->length` at byte offset 18 inside `struct ospf6_lsa_header`. The current first condition `p < OSPF6_MESSAGE_END(oh)` allows `ospf6_lsa_size()` to be called when as little as 1 byte remains, which means the 2-byte `length` field read at offset 18 goes past the end of the message buffer — the same class of out-of-bounds read the PR is trying to prevent.

Every other receive loop in this file guards with `p + sizeof(struct ospf6_lsa_header) <= OSPF6_MESSAGE_END(oh)` first (see `ospf6_lsack_recv` at line 1694, the two `lsreq` loops at lines 753 and 981). The first condition here should be changed to match that pattern:

`p + sizeof(struct ospf6_lsa_header) <= OSPF6_MESSAGE_END(oh)` before invoking `ospf6_lsa_size()`, followed by the new size check, followed by the existing end-of-buffer check.

This ensures the header can be safely read before inspecting its `length` field.

### Issue 2 of 2
ospf6d/ospf6_message.c:218-226
**`ospf6_lsupdate_print` still carries the pre-fix unsafe pattern**

The debug print loop here has the same `p < OSPF6_MESSAGE_END(oh) && p + ospf6_lsa_size(p) <= ...` structure that was left unpatched. A crafted LSU whose tail contains fewer than 20 bytes will cause `ospf6_lsa_size()` to read `header->length` at offset 18 out of bounds. While this path is gated on debug flags, the same well-formed packet that exercises `ospf6_lsupdate_recv` also goes through `ospf6_lsupdate_print` when debug logging is enabled, so the guard added to the receive loop should be applied here too.

_{Reviews (1): Last reviewed commit: "ospf6d: reject undersized LSAs in ospf6_..." | Re-trigger Greptile}

[greptile-apps]

Missing full-header guard before ospf6_lsa_size() call

ospf6_lsa_size() reads header->length at byte offset 18 inside struct ospf6_lsa_header. The current first condition p < OSPF6_MESSAGE_END(oh) allows ospf6_lsa_size() to be called when as little as 1 byte remains, which means the 2-byte length field read at offset 18 goes past the end of the message buffer — the same class of out-of-bounds read the PR is trying to prevent.

Every other receive loop in this file guards with p + sizeof(struct ospf6_lsa_header) <= OSPF6_MESSAGE_END(oh) first (see ospf6_lsack_recv at line 1694, the two lsreq loops at lines 753 and 981). The first condition here should be changed to match that pattern:

p + sizeof(struct ospf6_lsa_header) <= OSPF6_MESSAGE_END(oh) before invoking ospf6_lsa_size(), followed by the new size check, followed by the existing end-of-buffer check.

This ensures the header can be safely read before inspecting its length field.

Prompt To Fix With AI

This is a comment left during a code review.
Path: ospf6d/ospf6_message.c
Line: 1650-1655

Comment:
**Missing full-header guard before `ospf6_lsa_size()` call**

`ospf6_lsa_size()` reads `header->length` at byte offset 18 inside `struct ospf6_lsa_header`. The current first condition `p < OSPF6_MESSAGE_END(oh)` allows `ospf6_lsa_size()` to be called when as little as 1 byte remains, which means the 2-byte `length` field read at offset 18 goes past the end of the message buffer — the same class of out-of-bounds read the PR is trying to prevent.

Every other receive loop in this file guards with `p + sizeof(struct ospf6_lsa_header) <= OSPF6_MESSAGE_END(oh)` first (see `ospf6_lsack_recv` at line 1694, the two `lsreq` loops at lines 753 and 981). The first condition here should be changed to match that pattern:

`p + sizeof(struct ospf6_lsa_header) <= OSPF6_MESSAGE_END(oh)` before invoking `ospf6_lsa_size()`, followed by the new size check, followed by the existing end-of-buffer check.

This ensures the header can be safely read before inspecting its `length` field.

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