eigrpd: improve validation and error-handling in tlv parsing

[mjstapp]

Add more length validation before accessing packet data; add error-detection in several paths; try to skip unknown TLV types in more cases.

[greptile-apps]

Greptile Summary

This PR hardens the EIGRP TLV parsing loop across five packet-receive handlers (query, reply, siaquery, siareply, update) and the shared eigrp_read_ipv4_tlv helper. It addresses several classes of vulnerability that a malformed or adversarially crafted packet could trigger:

• eigrp_packet.c: eigrp_read_ipv4_tlv now validates a minimum of 25 bytes up front (covering the fixed header fields) and then re-validates the additional 1–4 bytes needed for the variable-length destination address based on prefix_length. The function returns NULL on failure.
• All five receive handlers: Added a 4-byte minimum check before each loop iteration so a partial TLV header cannot cause an over-read. NULL returns from eigrp_read_ipv4_tlv are now detected and cause the remainder of the packet to be skipped cleanly.
• eigrp_reply.c / eigrp_siaquery.c / eigrp_siareply.c: Unknown TLV types are now properly length-skipped rather than being silently ignored (which previously would have left the stream pointer un-advanced and produced an infinite loop or misaligned parsing).
• eigrp_query.c: Fixed the missing break in the default case and corrected a uint16_t underflow when length < 4.
• eigrp_update.c: Fixed a brace misalignment and added the missing stream_forward_getp before break on the "length exceeds remaining data" path.

The changes are consistent with the rest of the codebase's stream-API usage. One minor recurring pattern: the /* Invalid TLV - how to handle? */ comment that appears in all five NULL-TLV handlers is a leftover note from development; those paths would benefit from a zlog_warn call like the ones already present on the header-length guard paths.

Confidence Score: 5/5

• Safe to merge; changes are defensive hardening with no behavioural impact on well-formed packets.
• All the modified paths are error/malformed-packet branches that were previously unguarded. Well-formed packets follow the same happy path as before. The one style note (missing log on NULL-TLV paths) is non-blocking and does not affect correctness.
• No files require special attention.

Important Files Changed

Filename	Overview
eigrpd/eigrp_packet.c	Adds an upfront 25-byte readability guard and a prefix-length-dependent secondary guard before accessing destination address bytes in eigrp_read_ipv4_tlv; function now returns NULL on validation failure.
eigrpd/eigrp_query.c	Adds 4-byte TLV header guard, NULL-TLV check, and fixes the previously missing break in the default case; also fixes potential uint16_t underflow when length < 4. Leftover "how to handle?" comment and missing log on NULL path are minor gaps.
eigrpd/eigrp_reply.c	Replaces the infinite-loop-prone bare continue for unknown TLV types with a length-aware skip; adds 4-byte header guard and NULL-TLV check. Same missing log on NULL path as the other handlers.
eigrpd/eigrp_siaquery.c	Adds 4-byte header guard, NULL-TLV check, and proper length-based skip for non-IPv4-INT TLVs (previously they were silently ignored without advancing the stream pointer, risking an infinite loop).
eigrpd/eigrp_siareply.c	Identical structural improvements to eigrp_siaquery.c: header guard, NULL check, and proper unknown-TLV skip logic.
eigrpd/eigrp_update.c	Adds NULL-TLV guard; fixes a pre-existing indentation/brace misalignment in the default case and adds the missing stream_forward_getp before the break on the "length exceeds remaining data" path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Receive EIGRP packet] --> B{STREAM_READABLE >= 4?}
    B -- No --> B1[zlog_warn + skip remaining + break]
    B -- Yes --> C[stream_getw: read type]
    C --> D{type == EIGRP_TLV_IPv4_INT?}
    D -- No --> E[stream_getw: read length]
    E --> F{length < 4 OR readable < length-4?}
    F -- Yes --> F1[skip remaining + break]
    F -- No --> F2[stream_forward_getp length-4 + continue]
    D -- Yes --> G[stream rewind 2 bytes]
    G --> H[eigrp_read_ipv4_tlv]
    H --> I{STREAM_READABLE >= 25?}
    I -- No --> I1[return NULL]
    I -- Yes --> J[read fixed 25-byte header fields]
    J --> K{STREAM_READABLE >= bytes for prefix?}
    K -- No --> K1[free TLV + return NULL]
    K -- Yes --> L[read variable-length destination]
    L --> M[return TLV]
    M --> N{tlv == NULL?}
    N -- Yes --> N1[skip remaining + break]
    N -- No --> O[process TLV / free TLV]

Loading

Comments Outside Diff (1)

1. eigrpd/eigrp_query.c, line 107-110 (link)

   Leftover TODO comment without a log warning

   The comment /* Invalid TLV - how to handle? */ suggests unresolved intent. More importantly, there is no zlog_warn call here, unlike the 4-byte header guard just above which does emit a warning. A silent drop of the malformed TLV makes it very hard to diagnose protocol issues in the field.

   This same pattern appears in eigrpd/eigrp_reply.c:161-164, eigrpd/eigrp_siaquery.c:74-78, eigrpd/eigrp_siareply.c:73-77, and eigrpd/eigrp_update.c:295-299.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: eigrpd/eigrp_query.c
Line: 107-110

Comment:
**Leftover TODO comment without a log warning**

The comment `/* Invalid TLV - how to handle? */` suggests unresolved intent. More importantly, there is no `zlog_warn` call here, unlike the 4-byte header guard just above which does emit a warning. A silent drop of the malformed TLV makes it very hard to diagnose protocol issues in the field.

This same pattern appears in `eigrpd/eigrp_reply.c:161-164`, `eigrpd/eigrp_siaquery.c:74-78`, `eigrpd/eigrp_siareply.c:73-77`, and `eigrpd/eigrp_update.c:295-299`.

```suggestion
			if (tlv == NULL) {
				zlog_warn("Malformed packet: invalid IPv4 INT TLV, stopping TLV processing");
				stream_forward_getp(s, STREAM_READABLE(s));
				break;
			}
```

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

_{Reviews (2): Last reviewed commit: "eigrpd: skip unknown and ignored TLVs" | Re-trigger Greptile}

[mjstapp]

hmm, I think checkpatch is wrong in all three of the files it's complaining about: one branch does a "break", and the other does a "continue", and ... that seems ok to me?

[mjstapp]

I think greptile's comment about eigrp_query.c is valid- pushing a fix

[mjstapp]

@greptileai review

[Jafaral]

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

[mergify]

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

✅ Backports have been created

Details

• #21362 eigrpd: improve validation and error-handling in tlv parsing (backport #21316) has been created for branch stable/10.6
• #21363 eigrpd: improve validation and error-handling in tlv parsing (backport #21316) has been created for branch stable/10.5 but encountered conflicts

Cherry-pick of 284c966 has failed:

On branch mergify/bp/stable/10.5/pr-21316
Your branch is up to date with 'origin/stable/10.5'.

You are currently cherry-picking commit 284c966d5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   eigrpd/eigrp_packet.c
	modified:   eigrpd/eigrp_query.c
	modified:   eigrpd/eigrp_reply.c
	modified:   eigrpd/eigrp_siaquery.c
	modified:   eigrpd/eigrp_siareply.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   eigrpd/eigrp_update.c

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

• #21364 eigrpd: improve validation and error-handling in tlv parsing (backport #21316) has been created for branch stable/10.4 but encountered conflicts

Cherry-pick of 284c966 has failed:

On branch mergify/bp/stable/10.4/pr-21316
Your branch is up to date with 'origin/stable/10.4'.

You are currently cherry-picking commit 284c966d5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   eigrpd/eigrp_packet.c
	modified:   eigrpd/eigrp_query.c
	modified:   eigrpd/eigrp_reply.c
	modified:   eigrpd/eigrp_siaquery.c
	modified:   eigrpd/eigrp_siareply.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   eigrpd/eigrp_update.c

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

• #21365 eigrpd: improve validation and error-handling in tlv parsing (backport #21316) has been created for branch stable/10.3 but encountered conflicts

Cherry-pick of 284c966 has failed:

On branch mergify/bp/stable/10.3/pr-21316
Your branch is up to date with 'origin/stable/10.3'.

You are currently cherry-picking commit 284c966d5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   eigrpd/eigrp_query.c
	modified:   eigrpd/eigrp_reply.c
	modified:   eigrpd/eigrp_siaquery.c
	modified:   eigrpd/eigrp_siareply.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   eigrpd/eigrp_packet.c
	both modified:   eigrpd/eigrp_update.c

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

• #21366 eigrpd: improve validation and error-handling in tlv parsing (backport #21316) has been created for branch stable/10.2 but encountered conflicts

Cherry-pick of 284c966 has failed:

On branch mergify/bp/stable/10.2/pr-21316
Your branch is up to date with 'origin/stable/10.2'.

You are currently cherry-picking commit 284c966d5.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   eigrpd/eigrp_query.c
	modified:   eigrpd/eigrp_reply.c
	modified:   eigrpd/eigrp_siaquery.c
	modified:   eigrpd/eigrp_siareply.c

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   eigrpd/eigrp_packet.c
	both modified:   eigrpd/eigrp_update.c

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

[mjstapp]

small re-org to try to satisfy checkpatch...