From 7cb2b9f6cb44d5ce59c0c30138a20fc6c9c4eb0c Mon Sep 17 00:00:00 2001
From: Roland Shoemaker <bracewell@google.com>
Date: Wed, 10 Jan 2024 11:02:14 -0800
Subject: [PATCH 3/4] [Backport] net/mail: properly handle special characters
 in phrase and obs-phrase

Offering: Cloud Core Network
CVE: CVE-2024-24784
Reference: https://go-review.googlesource.com/c/go/+/566195

Fixes a couple of misalignments with RFC 5322 which introduce
significant diffs between (mostly) conformant parsers.

This change reverts the changes made in CL50911, which allowed certain
special RFC 5322 characters to appear unquoted in the "phrase" syntax.
It is unclear why this change was made in the first place, and created
a divergence from comformant parsers. In particular this resulted in
treating comments in display names incorrectly.

Additionally properly handle trailing malformed comments in the group
syntax.

Note: The upstream does not submit this change to go1.16 according to the rules of MinorReleases.
Corego2.x are based on go1.16.5. Therefore, it need to submit the change to corego2.x.

Edited-by: machangwang m00509938

For #65083
Fixes #65848

Change-Id: I00dddc044c6ae3381154e43236632604c390f672
Reviewed-on: https://go-review.googlesource.com/c/go/+/555596
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/566195
Reviewed-by: Carlos Amedee <carlos@golang.org>
Signed-off-by: Ma Chang Wang machangwang@huawei.com
---
 src/net/mail/message.go      | 30 +++++++++++++++------------
 src/net/mail/message_test.go | 40 ++++++++++++++++++++++++++----------
 2 files changed, 46 insertions(+), 24 deletions(-)

diff --git a/src/net/mail/message.go b/src/net/mail/message.go
index 09fb794005..f1d7e2f989 100644
--- a/src/net/mail/message.go
+++ b/src/net/mail/message.go
@@ -217,7 +217,7 @@ func (a *Address) String() string {
 	// Add quotes if needed
 	quoteLocal := false
 	for i, r := range local {
-		if isAtext(r, false, false) {
+		if isAtext(r, false) {
 			continue
 		}
 		if r == '.' {
@@ -381,7 +381,7 @@ func (p *addrParser) parseAddress(handleGroup bool) ([]*Address, error) {
 	if !p.consume('<') {
 		atext := true
 		for _, r := range displayName {
-			if !isAtext(r, true, false) {
+			if !isAtext(r, true) {
 				atext = false
 				break
 			}
@@ -416,7 +416,9 @@ func (p *addrParser) consumeGroupList() ([]*Address, error) {
 	// handle empty group.
 	p.skipSpace()
 	if p.consume(';') {
-		p.skipCFWS()
+		if !p.skipCFWS() {
+			return nil, errors.New("mail: misformatted parenthetical comment")
+		}
 		return group, nil
 	}
 
@@ -433,7 +435,9 @@ func (p *addrParser) consumeGroupList() ([]*Address, error) {
 			return nil, errors.New("mail: misformatted parenthetical comment")
 		}
 		if p.consume(';') {
-			p.skipCFWS()
+			if !p.skipCFWS() {
+				return nil, errors.New("mail: misformatted parenthetical comment")
+			}
 			break
 		}
 		if !p.consume(',') {
@@ -503,6 +507,12 @@ func (p *addrParser) consumePhrase() (phrase string, err error) {
 	var words []string
 	var isPrevEncoded bool
 	for {
+		// obs-phrase allows CFWS after one word
+		if len(words) > 0 {
+			if !p.skipCFWS() {
+				return "", errors.New("mail: misformatted parenthetical comment")
+			}
+		}
 		// word = atom / quoted-string
 		var word string
 		p.skipSpace()
@@ -598,7 +608,6 @@ Loop:
 // If dot is true, consumeAtom parses an RFC 5322 dot-atom instead.
 // If permissive is true, consumeAtom will not fail on:
 // - leading/trailing/double dots in the atom (see golang.org/issue/4938)
-// - special characters (RFC 5322 3.2.3) except '<', '>', ':' and '"' (see golang.org/issue/21018)
 func (p *addrParser) consumeAtom(dot bool, permissive bool) (atom string, err error) {
 	i := 0
 
@@ -609,7 +618,7 @@ Loop:
 		case size == 1 && r == utf8.RuneError:
 			return "", fmt.Errorf("mail: invalid utf-8 in address: %q", p.s)
 
-		case size == 0 || !isAtext(r, dot, permissive):
+		case size == 0 || !isAtext(r, dot):
 			break Loop
 
 		default:
@@ -763,18 +772,13 @@ func (e charsetError) Error() string {
 
 // isAtext reports whether r is an RFC 5322 atext character.
 // If dot is true, period is included.
-// If permissive is true, RFC 5322 3.2.3 specials is included,
-// except '<', '>', ':' and '"'.
-func isAtext(r rune, dot, permissive bool) bool {
+func isAtext(r rune, dot bool) bool {
 	switch r {
 	case '.':
 		return dot
 
 	// RFC 5322 3.2.3. specials
-	case '(', ')', '[', ']', ';', '@', '\\', ',':
-		return permissive
-
-	case '<', '>', '"', ':':
+	case '(', ')', '<', '>', '[', ']', ':', ';', '@', '\\', ',', '"': // RFC 5322 3.2.3. specials
 		return false
 	}
 	return isVchar(r)
diff --git a/src/net/mail/message_test.go b/src/net/mail/message_test.go
index 67e3643aeb..eeaa86e028 100644
--- a/src/net/mail/message_test.go
+++ b/src/net/mail/message_test.go
@@ -296,8 +296,11 @@ func TestAddressParsingError(t *testing.T) {
 		13: {"group not closed: null@example.com", "expected comma"},
 		14: {"group: first@example.com, second@example.com;", "group with multiple addresses"},
 		15: {"john.doe", "missing '@' or angle-addr"},
-		16: {"john.doe@", "no angle-addr"},
+		16: {"john.doe@", "missing '@' or angle-addr"},
 		17: {"John Doe@foo.bar", "no angle-addr"},
+		18: {" group: null@example.com; (asd", "misformatted parenthetical comment"},
+		19: {" group: ; (asd", "misformatted parenthetical comment"},
+		20: {`(John) Doe <jdoe@machine.example>`, "missing word in phrase:"},
 	}
 
 	for i, tc := range mustErrTestCases {
@@ -336,24 +339,19 @@ func TestAddressParsing(t *testing.T) {
 				Address: "john.q.public@example.com",
 			}},
 		},
-		{
-			`"John (middle) Doe" <jdoe@machine.example>`,
-			[]*Address{{
-				Name:    "John (middle) Doe",
-				Address: "jdoe@machine.example",
-			}},
-		},
+		// Comment in display name
 		{
 			`John (middle) Doe <jdoe@machine.example>`,
 			[]*Address{{
-				Name:    "John (middle) Doe",
+				Name:    "John Doe",
 				Address: "jdoe@machine.example",
 			}},
 		},
+		// Display name is quoted string, so comment is not a comment
 		{
-			`John !@M@! Doe <jdoe@machine.example>`,
+			`"John (middle) Doe" <jdoe@machine.example>`,
 			[]*Address{{
-				Name:    "John !@M@! Doe",
+				Name:    "John (middle) Doe",
 				Address: "jdoe@machine.example",
 			}},
 		},
@@ -688,6 +686,26 @@ func TestAddressParsing(t *testing.T) {
 				},
 			},
 		},
+		// Comment in group display name
+		{
+			`group (comment:): a@example.com, b@example.com;`,
+			[]*Address{
+				{
+					Address: "a@example.com",
+				},
+				{
+					Address: "b@example.com",
+				},
+			},
+		},
+		{
+			`x(:"):"@a.example;("@b.example;`,
+			[]*Address{
+				{
+					Address: `@a.example;(@b.example`,
+				},
+			},
+		},
 	}
 	for _, test := range tests {
 		if len(test.exp) == 1 {
-- 
2.33.0

