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 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-17 21:33:53 +03:00
co-authored by Claude Opus 5.5
parent 683fdafc6d
commit da2e8b2ffe
5 changed files with 40 additions and 5 deletions
+3
View File
@@ -18,6 +18,9 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
removed, so `RCPT TO:<"; from=<x> to=<y> …"@domain>` spelled a second removed, so `RCPT TO:<"; from=<x> to=<y> …"@domain>` spelled a second
sender and recipient there, and the parser took the first. It now takes sender and recipient there, and the parser took the first. It now takes
Postfix's own tail at the end of the line. 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 - **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 three `dmarc_*` maps root creates at the start were missing from the list
of paths where a symbolic link is removed first. of paths where a symbolic link is removed first.
+2 -2
View File
@@ -704,8 +704,8 @@ the argument.
tail may hold none of the neutered characters and each SASL field is one 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 word (a `.*` SASL tail would have let a quoted sender spell the whole
tail); TestInboundRecipientCannotWriteTheRow. tail); TestInboundRecipientCannotWriteTheRow.
- [ ] **M5.** Keep at most two `From:`/`Sender:` values; security.md:810 - [x] **M5.** Keep at most two `From:`/`Sender:` values; security.md:810
reworded. — **Opus**, 0.5 h. reworded. — **Opus**, 0.5 h. *Done 2026-10-17:* TestSenderKeepsTwoFromAndSenderLines.
- [ ] **A4.** Duplicate-cookie line through `loggedPath`. — **Sonnet**, 0.25 h. - [ ] **A4.** Duplicate-cookie line through `loggedPath`. — **Sonnet**, 0.25 h.
- [ ] **S6.** `smtpd_etrn_restrictions = reject`, `smtpd_discard_ehlo_keywords - [ ] **S6.** `smtpd_etrn_restrictions = reject`, `smtpd_discard_ehlo_keywords
= etrn` in postfix-config.sh; architecture.md § Postfix one line. Done when = etrn` in postfix-config.sh; architecture.md § Postfix one line. Done when
+4 -1
View File
@@ -824,7 +824,10 @@ deferred item from the roadmap.
`headerLimit`; `2.8.0`): it keeps no more than that and drops its list `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, 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. 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 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, 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 from strangers on port 25 - which none of the milters read; the panel
+7
View File
@@ -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) { 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. // 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)) { switch textproto.CanonicalMIMEHeaderKey(strings.TrimSpace(name)) {
case "From": case "From":
if len(s.froms) < 2 {
s.froms = append(s.froms, value) s.froms = append(s.froms, value)
}
case "Sender": case "Sender":
if len(s.senders) < 2 {
s.senders = append(s.senders, value) s.senders = append(s.senders, value)
}
case "Subject": case "Subject":
if s.subject == "" { if s.subject == "" {
s.subject = mailhdr.DecodeSubject(value) s.subject = mailhdr.DecodeSubject(value)
+22
View File
@@ -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"}, {"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"}, {"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"}, {"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"}, {"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"}, {"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, <a@shop.example.org>")}, false, "one address"}, {"an address before a list comma, then another", "shop", [][2]string{from("director@example.org, <a@shop.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))
}
}