From d262af74a358f3fc2b93f5f6ca7bb4795ae75da3 Mon Sep 17 00:00:00 2001 From: Dmitry Valter Date: Thu, 23 Apr 2020 22:31:57 +0300 Subject: [PATCH] textproto: check header characters Add checks for header field name according to RFC 6532 and disallow newline characters in field values. --- textproto/header.go | 40 +++++++++++++----- textproto/header_test.go | 91 +++++++++++++++++++++++++++++++++++----- 2 files changed, 111 insertions(+), 20 deletions(-) diff --git a/textproto/header.go b/textproto/header.go index 9dbb33ca..63ae8259 100644 --- a/textproto/header.go +++ b/textproto/header.go @@ -20,11 +20,22 @@ func newHeaderField(k, v string, b []byte) *headerField { return &headerField{k: textproto.CanonicalMIMEHeaderKey(k), v: v, b: b} } -func (f *headerField) raw() []byte { +func (f *headerField) raw() ([]byte, error) { if f.b != nil { - return f.b + return f.b, nil } else { - return []byte(formatHeaderField(f.k, f.v)) + for pos, ch := range f.k { + // check if character is a printable US-ASCII except ':' + if !(ch >= '!' && ch < ':' || ch > ':' && ch <= '~') { + return nil, fmt.Errorf("field name contains incorrect symbols (\\x%x at %v)", ch, pos) + } + } + + if pos := strings.IndexAny(f.v, "\r\n"); pos != -1 { + return nil, fmt.Errorf("field value contains \\r\\n (at %v)", pos) + } + + return []byte(formatHeaderField(f.k, f.v)), nil } } @@ -97,6 +108,9 @@ func (h *Header) AddRaw(kv []byte) { // Add adds the key, value pair to the header. It prepends to any existing // fields associated with key. +// +// Key and value should obey character requirements of RFC 6532. +// If you need to format/fold lines manually, use AddRaw func (h *Header) Add(k, v string) { k = textproto.CanonicalMIMEHeaderKey(k) @@ -126,10 +140,12 @@ func (h *Header) Get(k string) string { // // The returned slice should not be modified and becomes invalid when the // header is updated. -func (h *Header) Raw(k string) []byte { +// +// Error is returned if header contains incorrect characters (RFC 6532) +func (h *Header) Raw(k string) ([]byte, error) { fields := h.m[textproto.CanonicalMIMEHeaderKey(k)] if len(fields) == 0 { - return nil + return nil, nil } return fields[len(fields)-1].raw() } @@ -185,7 +201,7 @@ type HeaderFields interface { // Value returns the value of the current field. Value() string // Raw returns the raw current header field. See Header.Raw. - Raw() []byte + Raw() ([]byte, error) // Del deletes the current field. Del() // Len returns the amount of header fields in the subset of header iterated @@ -228,7 +244,7 @@ func (fs *headerFields) Value() string { return fs.field().v } -func (fs *headerFields) Raw() []byte { +func (fs *headerFields) Raw() ([]byte, error) { return fs.field().raw() } @@ -298,7 +314,7 @@ func (fs *headerFieldsByKey) Value() string { return fs.field().v } -func (fs *headerFieldsByKey) Raw() []byte { +func (fs *headerFieldsByKey) Raw() ([]byte, error) { return fs.field().raw() } @@ -628,8 +644,12 @@ func formatHeaderField(k, v string) string { func WriteHeader(w io.Writer, h Header) error { for i := len(h.l) - 1; i >= 0; i-- { f := h.l[i] - if _, err := w.Write(f.raw()); err != nil { - return err + if rawField, err := f.raw(); err == nil { + if _, err := w.Write(rawField); err != nil { + return err + } + } else { + return fmt.Errorf("failed to write header field #%v (%q): %w", len(h.l)-i, f.k, err) } } diff --git a/textproto/header_test.go b/textproto/header_test.go index 79c776ef..e6354b4a 100644 --- a/textproto/header_test.go +++ b/textproto/header_test.go @@ -429,16 +429,6 @@ var formatHeaderFieldTests = []struct { v: "InCaseOfVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongStringWeStillShouldComplyToTheHardLimitOf998Symbols", formatted: "Subject: InCaseOfVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryVeryLongStringWeStillSho\r\n uldComplyToTheHardLimitOf998Symbols\r\n", }, - { - k: "DKIM-Signature", - v: "v=1;\r\n h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version;\r\n d=example.org\r\n", - formatted: "Dkim-Signature: v=1;\r\n h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version;\r\n d=example.org\r\n", - }, - { - k: "DKIM-Signature", - v: "v=1; h=From; d=example.org; b=AuUoFEfDxTDkHlLXSZEpZj79LICEps6eda7W3deTVFOk4yAUoqOB4nujc7YopdG5dWLSdNg6x NAZpOPr+kHxt1IrE+NahM6L/LbvaHutKVdkLLkpVaVVQPzeRDI009SO2Il5Lu7rDNH6mZckBdrI x0orEtZV4bmp/YzhwvcubU4=\r\n", - formatted: "Dkim-Signature: v=1; h=From; d=example.org;\r\n b=AuUoFEfDxTDkHlLXSZEpZj79LICEps6eda7W3deTVFOk4yAUoqOB4nujc7YopdG5dWLSdNg6x\r\n NAZpOPr+kHxt1IrE+NahM6L/LbvaHutKVdkLLkpVaVVQPzeRDI009SO2Il5Lu7rDNH6mZckBdrI\r\n x0orEtZV4bmp/YzhwvcubU4=\r\n", - }, { k: "Bcc", v: "", @@ -465,3 +455,84 @@ func TestWriteHeader_continued(t *testing.T) { } } } + +var incorrectFormatHeaderFieldTests = []struct { + k, v string +}{ + { + k: "DKIM Signature", + v: "v=1; h=From; d=example.org; b=AuUoFEfDxTDkHlLXSZEpZj79LICEps6eda7W3deTVFOk4yAUoqOB4nujc7YopdG5dWLSdNg6x NAZpOPr+kHxt1IrE+NahM6L/LbvaHutKVdkLLkpVaVVQPzeRDI009SO2Il5Lu7rDNH6mZckBdrI x0orEtZV4bmp/YzhwvcubU4=\r\n", + }, + { + // Unicode, Cyrillic + k: "\u0417\u0430\u0433\u043e\u043b\u043e\u0432\u043e\u043a", + v: "Value", + }, + { + k: "Header:", + v: "Value", + }, + { + k: "DKIM-Signature", + v: "v=1;\r\n h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version;\r\n d=example.org\r\n", + }, + { + k: "DKIM-Signature", + v: "v=1;\n h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version; d=example.org", + }, + { + k: "DKIM-Signature", + v: "v=1;\r h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version; d=example.org", + }, +} + +func TestWriteHeader_failed(t *testing.T) { + for _, test := range incorrectFormatHeaderFieldTests { + var h Header + h.Add(test.k, test.v) + + var b bytes.Buffer + if err := WriteHeader(&b, h); err == nil { + t.Errorf("Expected header \n%v: %v\n to be incorrect, but it was accepted", test.k, test.v) + } + } +} + +var incorrectFormatMultipleHeaderFieldTests = []struct { + k1, k2, v1, v2 string +}{ + { + // Incorrect first + k1: "DKIM Signature", + v1: "v=1; h=From; d=example.org; b=AuUoFEfDxTDkHlLXSZEpZj79LICEps6eda7W3deTVFOk4yAUoqOB4nujc7YopdG5dWLSdNg6x NAZpOPr+kHxt1IrE+NahM6L/LbvaHutKVdkLLkpVaVVQPzeRDI009SO2Il5Lu7rDNH6mZckBdrI x0orEtZV4bmp/YzhwvcubU4=\r\n", + k2: "From", + v2: "alice@example.com", + }, + { + // Incorrect both + k1: "\u0417\u0430\u0433\u043e\u043b\u043e\u0432\u043e\u043a", + v1: "Value", + k2: "Header:", + v2: "Value", + }, + { + // Incorrect second + k1: "DKIM-Signature", + v1: "v=1; h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version; d=example.org", + k2: "DKIM-Signature", + v2: "v=1;\r\n h=From:To:Reply-To:Subject:Message-ID:References:In-Reply-To:MIME-Version;\r\n d=example.org\r\n", + }, +} + +func TestWriteHeader_failed_multiple(t *testing.T) { + for _, test := range incorrectFormatMultipleHeaderFieldTests { + var h Header + h.Add(test.k1, test.v1) + h.Add(test.k2, test.v2) + + var b bytes.Buffer + if err := WriteHeader(&b, h); err == nil { + t.Errorf("Expected headers \n%v: %v\n%v: %v\n to be incorrect, but it was accepted", test.k1, test.v2, test.k2, test.v2) + } + } +} \ No newline at end of file