From da2e8b2ffedb062ec03663f8108b7f9adc947279 Mon Sep 17 00:00:00 2001 From: Mix Date: Sat, 17 Oct 2026 21:33:53 +0300 Subject: [PATCH] milter: the sender check keeps two From: and two Sender: values (audit M5) Security audit 2026-10-17, finding M5 (low), stage 0. senderSession.Header appended every From:/Sender: value; Postfix bounds one header at 60 000 bytes but not their number, and the signer's 1 MiB cap runs after the sender check, so an application could make it hold megabytes per connection (the reader's test: a hundred lines, 6 MB). judge needs only none / one / more, so two of each are kept; the refusal text joins what is kept, as before. Tests: "a hundred From: lines" in the judgement table (refused, more than one From: line); TestSenderKeepsTwoFromAndSenderLines. go test ./internal/milter. Docs: security.md (the sentence "a few headers each"); CHANGELOG; plan. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 3 +++ docs/security-audit-2026-10-17.md | 4 ++-- docs/security.md | 5 ++++- internal/milter/sender.go | 11 +++++++++-- internal/milter/sender_test.go | 22 ++++++++++++++++++++++ 5 files changed, 40 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 732dd06..57f1dc0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version removed, so `RCPT TO:<"; from= to= …"@domain>` spelled a second sender and recipient there, and the parser took the first. It now takes Postfix's own tail at the end of the line. +- **The sender check keeps two `From:` and two `Sender:` lines** (audit M5), + not every one an application sends: the judgement needs no more, and a + hundred long lines were megabytes held per connection. - **The entrypoint follows no link at the DMARC maps** (audit I2): the three `dmarc_*` maps root creates at the start were missing from the list of paths where a symbolic link is removed first. diff --git a/docs/security-audit-2026-10-17.md b/docs/security-audit-2026-10-17.md index e1f7e26..5c7a4e6 100644 --- a/docs/security-audit-2026-10-17.md +++ b/docs/security-audit-2026-10-17.md @@ -704,8 +704,8 @@ the argument. tail may hold none of the neutered characters and each SASL field is one word (a `.*` SASL tail would have let a quoted sender spell the whole tail); TestInboundRecipientCannotWriteTheRow. -- [ ] **M5.** Keep at most two `From:`/`Sender:` values; security.md:810 - reworded. — **Opus**, 0.5 h. +- [x] **M5.** Keep at most two `From:`/`Sender:` values; security.md:810 + reworded. — **Opus**, 0.5 h. *Done 2026-10-17:* TestSenderKeepsTwoFromAndSenderLines. - [ ] **A4.** Duplicate-cookie line through `loggedPath`. — **Sonnet**, 0.25 h. - [ ] **S6.** `smtpd_etrn_restrictions = reject`, `smtpd_discard_ehlo_keywords = etrn` in postfix-config.sh; architecture.md § Postfix one line. Done when diff --git a/docs/security.md b/docs/security.md index 6c9087c..81f02f9 100644 --- a/docs/security.md +++ b/docs/security.md @@ -824,7 +824,10 @@ deferred item from the roadmap. `headerLimit`; `2.8.0`): it keeps no more than that and drops its list once the block it signs is built. No mail program writes such a section, and the refusal is permanent, so a sender is told rather than retried. - The other milters keep a few headers each (From, Sender, Subject). + The other milters keep a few headers each: the sender check two `From:` + and two `Sender:` values at most - it needs only none, one or more - and + the first Subject (until `2.14.4` it kept every `From:`/`Sender:` line, + which Postfix bounds in length but not in number; audit 2026-10-17, M5). Until `2.8.0` the milter library itself added every header of a message to a map of its own in each milter - up to the port's message size limit, from strangers on port 25 - which none of the milters read; the panel diff --git a/internal/milter/sender.go b/internal/milter/sender.go index 748b509..4f39d93 100644 --- a/internal/milter/sender.go +++ b/internal/milter/sender.go @@ -309,11 +309,18 @@ func (s *senderSession) RcptTo(rcpt string, m *milter.Modifier) (milter.Response func (s *senderSession) Header(name, value string, m *milter.Modifier) (milter.Response, error) { // The name is trimmed: the obsolete syntax allows blanks before the colon. + // Two of each are kept: the judgement needs only none, one or more, and + // Postfix bounds a header's length but not their number (audit + // 2026-10-17, M5: a hundred 60 000-byte From: lines were 6 MB held). switch textproto.CanonicalMIMEHeaderKey(strings.TrimSpace(name)) { case "From": - s.froms = append(s.froms, value) + if len(s.froms) < 2 { + s.froms = append(s.froms, value) + } case "Sender": - s.senders = append(s.senders, value) + if len(s.senders) < 2 { + s.senders = append(s.senders, value) + } case "Subject": if s.subject == "" { s.subject = mailhdr.DecodeSubject(value) diff --git a/internal/milter/sender_test.go b/internal/milter/sender_test.go index 465f904..bef6739 100644 --- a/internal/milter/sender_test.go +++ b/internal/milter/sender_test.go @@ -132,6 +132,7 @@ func TestSenderCheck(t *testing.T) { {"a subdomain of its own", "shop", [][2]string{from("a@sub.shop.example.org")}, false, "not an address this application sends as"}, {"no From: line", "shop", [][2]string{{"Subject", "hello"}}, false, "no From: line"}, {"two From: lines", "shop", [][2]string{from("a@shop.example.org"), from("director@example.org")}, false, "more than one From: line"}, + {"a hundred From: lines", "shop", manyFroms(100), false, "more than one From: line"}, {"two addresses in one line", "shop", [][2]string{from("a@shop.example.org, director@example.org")}, false, "one address"}, {"not an address", "shop", [][2]string{from("the shop")}, false, "as a mail program reads it"}, {"an address before a list comma, then another", "shop", [][2]string{from("director@example.org, ")}, false, "one address"}, @@ -460,3 +461,24 @@ func TestSenderLineHeldToTheSameRule(t *testing.T) { }) } } + +// manyFroms is n From: lines; the sender check keeps two of them (audit +// 2026-10-17, M5). +func manyFroms(n int) [][2]string { + out := make([][2]string, n) + for i := range out { + out[i] = [2]string{"From", "a@shop.example.org"} + } + return out +} + +func TestSenderKeepsTwoFromAndSenderLines(t *testing.T) { + s := &senderSession{} + for i := 0; i < 100; i++ { + s.Header("From", "a@shop.example.org", nil) + s.Header("Sender", "b@shop.example.org", nil) + } + if len(s.froms) != 2 || len(s.senders) != 2 { + t.Errorf("kept %d From: and %d Sender: values, want 2 and 2", len(s.froms), len(s.senders)) + } +}