Skip to content

Commit

Permalink
textproto: check header characters
Browse files Browse the repository at this point in the history
Add checks for header field name according to RFC 6532 and disallow newline
characters in field values.
  • Loading branch information
dvalter authored and emersion committed May 28, 2020
1 parent 5b97b1b commit d262af7
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 20 deletions.
40 changes: 30 additions & 10 deletions textproto/header.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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()
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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)
}
}

Expand Down
91 changes: 81 additions & 10 deletions textproto/header_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
Expand All @@ -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)
}
}
}

0 comments on commit d262af7

Please sign in to comment.