Prevent corrupted SMTP headers when MailAddress.DisplayName contains CR/LF

[Copilot]

MailAddress accepted display names containing \r/\n and SmtpClient embedded them verbatim into From:/To: headers, breaking header structure and SMTP framing. Typical source: a trailing newline left over from parsing upstream input.

Changes in MailAddress.TryParse (src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs):

• Caller-supplied display name: reject CR/LF by throwing FormatException, matching the existing convention for MailMessage.Subject (MailBnfHelper.HasCROrLF → MailSubjectInvalidFormat). TryCreate returns false.
• Display name parsed out of the address string: unfold per RFC 5322 §3.2.4 by stripping CR/LF from the stored value. This preserves the previously-accepted folded input "display \r\n name" <"folding"@domain.com> while ensuring re-encoding produces a clean header.
• New StripCRLF helper (guarded by an HasCROrLF pre-check, so allocation only occurs when needed).

// Now throws FormatException instead of producing a broken From: header
new MailAddress("user@example.com", "Display Name\r\n");

Test coverage added in MailAddressDisplayNameTest.MailAddress_Ctor_DisplayNameContainsCRLF_Throws for CR, LF, and CRLF in leading/middle/trailing positions and inside bounding quotes, against both the constructor and TryCreate.

[dotnet-policy-service]

Tagging subscribers to this area: @karelz, @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR hardens System.Net.Mail against CR/LF injection/corruption via MailAddress.DisplayName, preventing malformed From:/To: header output when addresses are serialized for SMTP.

Changes:

• Add CR/LF rejection for caller-supplied displayName in MailAddress.TryParse (throwing for constructors; TryCreate returns false).
• Strip CR/LF from display names parsed out of the full address string (unfolding) via a new StripCRLF helper.
• Add unit tests covering CR, LF, and CRLF in caller-supplied display names for both MailAddress constructors and TryCreate.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/libraries/System.Net.Mail/src/System/Net/Mail/MailAddress.cs	Reject CR/LF in caller-supplied display names; strip CR/LF from parsed display names to avoid header corruption.
src/libraries/System.Net.Mail/tests/Unit/MailAddressTests/MailAddressDisplayNameTest.cs	Add unit tests asserting CR/LF in caller-supplied display names throws / fails TryCreate.

[mrek-msft]

@copilot consider fixing on other places or reusing logic. Please take in account recently merged fix #130175.

[Copilot]

Addressed in f12da96. The branch now aligns with #130175 by rejecting CR/LF at the parser level for address strings (covering ParseMultipleAddresses/MailAddressCollection.Add and MailMessage construction), while keeping the separate rejection for caller-supplied displayName. I also removed the earlier unfold path and updated the affected unit/functional tests accordingly.

[github-actions]

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "f12da9684842c6d245ceaa43a90a394ae6908023",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "58ecafbefc41532278f7e98e8accabffa1040a56",
  "last_reviewed_commit": "f12da9684842c6d245ceaa43a90a394ae6908023",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "58ecafbefc41532278f7e98e8accabffa1040a56",
  "last_recorded_worker_run_id": "29680697041",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "f12da9684842c6d245ceaa43a90a394ae6908023",
      "review_id": 4730523109
    }
  ]
}

[github-actions]

Holistic Review

Motivation: MailAddress accepted display names and address strings containing bare CR/LF characters, which SmtpClient then embedded verbatim into From:/To: headers. This corrupts header structure and enables SMTP header/command injection (CRLF injection). Rejecting these inputs is a well-motivated correctness and security fix, and it aligns MailAddress with the existing MailBnfHelper.HasCROrLF convention already used for MailMessage.Subject, ContentType, and ContentDisposition.

Approach: Two guard points are added. (1) In MailAddress.TryParse, a caller-supplied displayName containing CR/LF now throws FormatException (or returns false from TryCreate). (2) In MailAddressParser.TryParseAddress, the full input string is scanned once for CR/LF on the initial reverse-parse call (guarded by index == data.Length - 1 so ParseMultipleAddresses keeps the scan O(n) overall). Tests are updated to move previously-valid folded inputs into the invalid set and to add focused CR/LF coverage for the constructor, TryCreate, TryParseAddress, and ParseMultipleAddresses.

The implementation is clean and the placement is sensible: the parser-level scan gives a single choke point covering all public entry points, and the HasCROrLF span check is cheap. Test coverage is thorough (leading/middle/trailing/quoted positions, both throwing and non-throwing overloads, and multi-address lists).

Summary: The change is correct and improves security. Two points warrant attention before merge, both non-blocking:

1. Behavior/breaking change: This is a deliberate behavioral change. Inputs that were previously accepted via RFC 5322 folding — e.g. "display \r\n name" <"folding"@Domain.com>, "test\r\n test"@mail.com, and folded domain literals like testuser@[mail.com \r\n ] — now throw FormatException (the test data moves them from the valid set to the invalid set). Rejecting them is defensible for security, but this is an observable compatibility change for callers that parse already-folded header values. It should be flagged for a breaking-change note / API review, and consider whether unfolding (rather than rejecting) folded whitespace originating from parsed input is preferable for round-tripping scenarios.

2. PR description is stale relative to the code: The description states the parsed-out display name is unfolded by a new StripCRLF helper that strips CR/LF, preserving folded input. The committed code contains no StripCRLF helper and instead rejects CR/LF at the parser level. The description should be updated to match the actual reject-based implementation to avoid confusion for reviewers and future readers.

No functional defects were found in the diff itself; the logic, guard conditions, and tests are sound. Verdict: approve with the above follow-ups.

Detailed Findings

• Behavior change surfacing previously-valid folded addresses as errors (MailAddressParser.cs, MailAddressParsingTest.cs): See Summary point 1. Cross-cutting rather than tied to a single changed line; recommend a breaking-change doc entry and confirmation that rejecting (vs. unfolding) is the intended contract.
• Stale PR description (StripCRLF/unfold vs. reject): See Summary point 2. Documentation-only.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 71.6 AIC · ⌖ 10.7 AIC · ⊞ 10K