Fix buffer overflow in enc28j60_lwip_xmit()

[orbisai0security]

Description

• Add bounds check in enc28j60_lwip_xmit() to prevent buffer overflow when traversing the pbuf chain.
• If the accumulated payload length exceeds the allocated buffer size, the function returns ERR_BUF and frees the temporary pbuf.

Motivation and Context

• The memcpy in the pbuf traversal loop could write beyond the allocated buffer if individual pbuf segment lengths sum to more than the expected packet length, causing a heap buffer overflow.
• On embedded devices without ASLR, this overflow is reliably exploitable from the local network segment.
• This is a security fix for the ENC28J60 Ethernet driver.

How Has This Been Tested?

• Code review verified the bounds check correctly validates idx + pPBuf->len against the allocated buffer size (length + 2).
• The fix preserves existing behavior for valid packets where total payload equals tot_len.

Types of changes

• Improvement (non-breaking change that improves a feature, code or algorithm)
• Bug fix (non-breaking change which fixes an issue with code or algorithm)
• New feature (non-breaking change which adds functionality to code)
• Breaking change (fix or feature that would cause existing functionality to change)
• Config and build (change in the configuration and build system, has no impact on code or features)
• Dev Containers (changes related with Dev Containers, has no impact on code or features)
• Dependencies/declarations (update dependencies or assembly declarations and changes associated, has no impact on code or features)
• Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

• My code follows the code style of this project (only if there are changes in source code).
• My changes require an update to the documentation (there are changes that require the docs website to be updated).
• I have updated the documentation accordingly (the changes require an update on the docs in this repo).
• I have read the CONTRIBUTING document.
• I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: f94d8719-ce43-4c4e-80a7-316e6b77b1cd

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The PR adds a defensive bounds check to the enc28j60_lwip_xmit() transmit function. Before copying the next pbuf segment into the temporary transmit buffer, the code now validates that appending it would not exceed the allocated capacity; if it would, the buffer is freed and an error is returned.

Changes

Transmit Buffer Bounds Check

Layer / File(s)	Summary
Buffer overflow protection in transmit loop src/DeviceInterfaces/Network/Enc28j60/enc28j60_lwip.cpp	Added a per-segment bounds check in the payload copy loop: if appending the next pPBuf would exceed buffer capacity, pTmp is freed and ERR_BUF is returned to prevent buffer overrun.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Fix buffer overflow in enc28j60_lwip_xmit()' is concise (43 characters), descriptive, directly summarizes the main security fix being implemented, and complies with all stated requirements.
Description check	✅ Passed	The description clearly explains the bounds check addition, the security vulnerability being fixed, testing approach, and motivation—all directly related to the changeset in enc28j60_lwip.cpp.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[orbisai0security]

@dotnet-policy-service agree

[josesimoes]

@orbisai0security thank you for reporting this and providing a fix. Can you please isolate the change in enc28j60_lwip.cpp (I suppose that's the key fix) and rebase the PR? That should be it.

[orbisai0security]

@orbisai0security thank you for reporting this and providing a fix. Can you please isolate the change in enc28j60_lwip.cpp (I suppose that's the key fix) and rebase the PR? That should be it.

@josesimoes I've isolated the changes and also rebased the PR. Pls review.

[josesimoes]

Perfect! Thank you very much for finding this and providing a fix.

[josesimoes]

@coderabbitai review

[josesimoes]

/azp run

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/DeviceInterfaces/Network/Enc28j60/enc28j60_lwip.cpp`:
- Around line 609-613: The early return that returns ERR_BUF after
pbuf_free(pTmp) skips the GLOBAL_UNLOCK() and can deadlock; before returning
ERR_BUF (and likewise before other early returns that return ERR_ARG/ERR_MEM in
the same function), call GLOBAL_UNLOCK() to release the global lock, then free
the pbuf and return the error code—update the branch using pTmp/pPBuf (the same
pattern as the existing fixes for the ERR_ARG and ERR_MEM returns) so every
early exit invokes GLOBAL_UNLOCK() prior to returning.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Central YAML (inherited)

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8cac009a-6e58-40f3-b369-f9fac7ae3eeb

📥 Commits

Reviewing files that changed from the base of the PR and between 5546446 and dd4bd1c.

📒 Files selected for processing (1)

• src/DeviceInterfaces/Network/Enc28j60/enc28j60_lwip.cpp

[josesimoes]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 2 pipeline(s).

[nfbot]

@orbisai0security thank you again for your contribution! 🙏😄

.NET nanoFramework is all about community involvement, and no contribution is too small.
We would like to invite you to join the project's Contributors list.

Please edit it and add an entry with your GitHub username in the appropriate location (names are sorted alphabetically):

  <tr>
    <td><img src="https://github.com/orbisai0security.png?size=50" height="50" width="50" ></td>
    <td><a href="https://github.com/orbisai0security"></a></td>
  </tr>

(Feel free to adjust your name if it's not correct)

[orbisai0security]

I analyzed your request and ran the commands, but no file changes were produced. This can happen when:

• The requested changes are already present in the code
• The change instructions weren't specific enough for me to identify the right modifications

Could you provide more specific instructions about which files and lines to change?