fix: enforce relay control and message size limit - #4
Merged
Conversation
handleRcpt accepted RCPT TO for any recipient with no auth or locality check (open relay); handleData never consulted maxMessageSize while buffering the DATA body (unbounded memory growth). Adds a localDomains- based relay check (unauthenticated sessions may only deliver to a domain this server hosts; authenticated sessions may relay anywhere, matching standard MSA/MX behavior) and enforces maxMessageSize while reading DATA, draining without buffering past the terminator on overflow so the connection stays protocol-consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to security/003-followup — deep-review-003's SMTP open-relay and DATA-size-DoS Design-Critical/High findings were reported fixed by that run's predicates, but both predicates just grepped for strings (
LocalDomains|relay|s.auth,maxMessageSize) that were already present pre-fix (a field declared and defaulted, never enforced). Closes both for real:handleRcptacceptedRCPT TOfor any recipient with no auth or locality check (open relay). Now: an unauthenticated session may only deliver to a domain this server hosts (localDomains); an authenticated session may relay anywhere (standard MSA/MX behavior).handleDatanever consultedmaxMessageSizewhile buffering the DATA body (unbounded memory growth). Now enforced while reading, draining without buffering past the terminator on overflow so the connection stays protocol-consistent (a response is only valid after the DATA terminator per RFC 5321).Note: this repo (like the rest of the email stack) is not deployed to production per deep-review-003, and has no CI test/lint workflow configured at all — flagging as pre-existing, out of scope here.
Test plan
GOWORK=off go build ./...,go vet ./...clean, full suite greenTestServerMessageDelivery/TestSessionHandleFullTransactionupdated to configure a local domain, matching realistic non-authenticated inbound delivery