DHCP: Santize messages from servers for output

[rsmarples]

They could contain anything!

Reported by NVIDIA Project Vanessa

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7dcf5176-5e65-49cc-bc8b-052b5c1255ea

📥 Commits

Reviewing files that changed from the base of the PR and between b6a3039 and db45579.

📒 Files selected for processing (2)

• src/dhcp.c
• src/dhcp6.c

🚧 Files skipped from review as they are similar to previous changes (1)

• src/dhcp.c

---

Walkthrough

get_option_string() in dhcp.c and dhcp6_checkstatusok in dhcp6.c now allocate worst-case-sized buffers and call print_string() to encode DHCP option payloads instead of doing raw memcpy with manual NUL termination. The log_dhcp() NAK path and dhcp_handledhcp() NAK path are simplified to remove redundant intermediate buffers and extra log calls.

Changes

DHCP Option String Encoding and NAK Logging

Layer / File(s)	Summary
String encoding in get_option_string() and dhcp6_checkstatusok src/dhcp.c, src/dhcp6.c	get_option_string() computes a larger buffer and calls print_string() instead of memcpy+NUL. dhcp6_checkstatusok introduces a status_code fallback label and uses print_string() for trailing message bytes, replacing a direct malloc(mlen+1) copy and returning gracefully on failure.
NAK log path consolidation src/dhcp.c	log_dhcp() assigns the DHO_MESSAGE string directly from get_option_string() for the "NAK:" case, removing the prior temporary buffer and re-print step. dhcp_handledhcp() drops the separate get_option_string()/logwarnx() calls and relies solely on LOGDHCP(LOG_WARNING, "NAK:").

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'DHCP: Sanitize messages from servers for output' directly and specifically describes the main objective of the changeset: sanitizing DHCP server messages before output.
Description check	✅ Passed	The description is related to the changeset, explaining that server messages could contain arbitrary content that needs sanitization, though it lacks specific technical detail.
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.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dhcp_msg

---

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.}

[coderabbitai]

Actionable comments posted: 2

🤖 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/dhcp.c`:
- Around line 506-509: The print_string call in the DHCP message handling code
uses OT_STRING mode which allows unescaped control characters to pass through,
creating a log injection vulnerability. Change the OT_STRING argument in the
print_string function call to use escaped output mode (typically
OT_ESCAPED_STRING) to properly escape control bytes like newlines, carriage
returns, and escape sequences. Additionally, add error handling to detect and
manage any encoding failures that may result from using escaped mode.

In `@src/dhcp6.c`:
- Around line 2141-2147: The print_string function call for the DHCPv6 status
message is using OT_STRING which allows ASCII control characters to be emitted
directly. Change the OT_STRING parameter to OT_ESCSTRING to ensure proper
escaping of status-message bytes. Additionally, add error handling after the
print_string call to check if the encoding failed, and if so, use goto to jump
to the status_code label as a fallback instead of blindly assigning sbuf to
status.

🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 424ac90f-d9dc-4b41-8b6b-a31d96c619c5

📥 Commits

Reviewing files that changed from the base of the PR and between 26e4325 and 66f3378.

📒 Files selected for processing (2)

• src/dhcp.c
• src/dhcp6.c