Fixed possible recursion crash when parsing mailbox groups.

8 files changed, 82 insertions, 20 deletions

diff --git a/src/vmime/address.cpp b/src/vmime/address.cpp
index 3bc434fa..5333ae52 100644
--- a/src/vmime/address.cpp
+++ b/src/vmime/address.cpp
@@ -71,6 +71,7 @@ shared_ptr <address> address::parseNext(
 	const size_t position,
 	const size_t end,
 	size_t* newPosition,
+	const bool allowGroup,
 	bool *isLastAddressOfGroup
 ) {
 
@@ -203,13 +204,16 @@ shared_ptr <address> address::parseNext(
 		}
 	}
 
-	if (newPosition) {
+	size_t newPos;
 
-		if (pos == end) {
-			*newPosition = end;
-		} else {
-			*newPosition = pos + 1;  // ',' or ';'
-		}
+	if (pos == end) {
+		newPos = end;
+	} else {
+		newPos = pos + 1;  // ',' or ';'
+	}
+
+	if (newPosition) {
+		*newPosition = newPos;
 	}
 
 	// Parse extracted address (mailbox or group)
@@ -218,13 +222,33 @@ shared_ptr <address> address::parseNext(
 		shared_ptr <address> parsedAddress;
 
 		if (isGroup) {
-			parsedAddress = make_shared <mailboxGroup>();
+
+			if (allowGroup) {
+
+				parsedAddress = make_shared <mailboxGroup>();
+
+			} else {  // group not allowed in group, ignore group and continue parsing
+
+				return parseNext(
+					ctx,
+					buffer,
+					newPos,
+					end,
+					newPosition,
+					/* allowGroup */ false,
+					isLastAddressOfGroup
+				);
+			}
+
 		} else {
+
 			parsedAddress = make_shared <mailbox>();
 		}
 
-		parsedAddress->parse(ctx, buffer, start, pos, NULL);
-		parsedAddress->setParsedBounds(start, pos);
+		if (parsedAddress) {
+			parsedAddress->parse(ctx, buffer, start, pos, NULL);
+			parsedAddress->setParsedBounds(start, pos);
+		}
 
 		return parsedAddress;
 	}
diff --git a/src/vmime/address.hpp b/src/vmime/address.hpp
index f83f2381..347eb500 100644
--- a/src/vmime/address.hpp
+++ b/src/vmime/address.hpp
@@ -78,6 +78,7 @@ public:
 		const size_t position,
 		const size_t end,
 		size_t* newPosition,
+		const bool allowGroup,
 		bool *isLastAddressOfGroup
 	);
 };
diff --git a/src/vmime/addressList.cpp b/src/vmime/addressList.cpp
index 03c9e8f4..ad6ce915 100644
--- a/src/vmime/addressList.cpp
+++ b/src/vmime/addressList.cpp
@@ -63,7 +63,7 @@ void addressList::parseImpl(
 
 	while (pos < end) {
 
-		shared_ptr <address> parsedAddress = address::parseNext(ctx, buffer, pos, end, &pos, NULL);
+		shared_ptr <address> parsedAddress = address::parseNext(ctx, buffer, pos, end, &pos, /* allowGroup */ true, NULL);
 
 		if (parsedAddress) {
 			m_list.push_back(parsedAddress);
diff --git a/src/vmime/mailboxField.cpp b/src/vmime/mailboxField.cpp
index 82ef5921..84c8f3d5 100644
--- a/src/vmime/mailboxField.cpp
+++ b/src/vmime/mailboxField.cpp
@@ -56,7 +56,7 @@ void mailboxField::parse(
 	// may have more than one address specified (even if this field
 	// should contain only one). We are never too much careful...
 	shared_ptr <address> parsedAddress = address::parseNext(
-		ctx, buffer, position, end, newPosition, NULL
+		ctx, buffer, position, end, newPosition, /* allowGroup */ true, NULL
 	);
 
 	if (parsedAddress) {
diff --git a/src/vmime/mailboxGroup.cpp b/src/vmime/mailboxGroup.cpp
index 38a6e35e..3baa3329 100644
--- a/src/vmime/mailboxGroup.cpp
+++ b/src/vmime/mailboxGroup.cpp
@@ -87,19 +87,13 @@ void mailboxGroup::parseImpl(
 	while (pos < end && !isLastAddressOfGroup) {
 
 		shared_ptr <address> parsedAddress =
-			address::parseNext(ctx, buffer, pos, end, &pos, &isLastAddressOfGroup);
+			address::parseNext(ctx, buffer, pos, end, &pos, /* allowGroup */ false, &isLastAddressOfGroup);
 
 		if (parsedAddress) {
 
 			if (parsedAddress->isGroup()) {
 
-				shared_ptr <mailboxGroup> group = dynamicCast <mailboxGroup>(parsedAddress);
-
-				// Sub-groups are not allowed in mailbox groups: so, we add all
-				// the contents of the sub-group into this group...
-				for (size_t i = 0 ; i < group->getMailboxCount() ; ++i) {
-					m_list.push_back(vmime::clone(group->getMailboxAt(i)));
-				}
+				// Should not happen
 
 			} else {
 
diff --git a/src/vmime/mailboxList.cpp b/src/vmime/mailboxList.cpp
index e7aba81b..84c06d83 100644
--- a/src/vmime/mailboxList.cpp
+++ b/src/vmime/mailboxList.cpp
@@ -204,7 +204,7 @@ void mailboxList::parseImpl(
 
 	while (pos < end) {
 
-		shared_ptr <address> parsedAddress = address::parseNext(ctx, buffer, pos, end, &pos, NULL);
+		shared_ptr <address> parsedAddress = address::parseNext(ctx, buffer, pos, end, &pos, /* allowGroup */ true, NULL);
 
 		if (parsedAddress) {
 
diff --git a/tests/parser/mailboxGroupTest.cpp b/tests/parser/mailboxGroupTest.cpp
index 18ff425b..00a7baad 100644
--- a/tests/parser/mailboxGroupTest.cpp
+++ b/tests/parser/mailboxGroupTest.cpp
@@ -32,6 +32,8 @@ VMIME_TEST_SUITE_BEGIN(mailboxGroupTest)
 		VMIME_TEST(testParseExtraChars)
 		VMIME_TEST(testEmptyGroup)
 		VMIME_TEST(testEncodedEmptyGroup)
+		VMIME_TEST(testGroupInGroup)
+		VMIME_TEST(testBrokenGroup)
 	VMIME_TEST_LIST_END
 
 
@@ -104,4 +106,30 @@ VMIME_TEST_SUITE_BEGIN(mailboxGroupTest)
 		VASSERT_EQ("count", 0, mgrp.getMailboxCount());
 	}
 
+	void testGroupInGroup() {
+
+		vmime::mailboxGroup mgrp;
+		mgrp.parse("group1:[email protected],group2:[email protected];,[email protected];");
+
+		VASSERT_EQ("name", "group1", mgrp.getName().getWholeBuffer());
+		VASSERT_EQ("count", 2, mgrp.getMailboxCount());
+		VASSERT_EQ("mbox1", "[email protected]", mgrp.getMailboxAt(0)->getEmail());
+		VASSERT_EQ("mbox2", "[email protected]", mgrp.getMailboxAt(1)->getEmail());
+	}
+
+	void testBrokenGroup() {
+
+		std::string bad(":,");
+
+		for (int i = 0 ; i < 10 ; ++i) {
+			bad = bad + bad;
+		}
+
+		vmime::mailboxGroup mgrp;
+		mgrp.parse(bad);
+
+		VASSERT_EQ("name", "", mgrp.getName().getWholeBuffer());
+		VASSERT_EQ("count", 0, mgrp.getMailboxCount());
+	}
+
 VMIME_TEST_SUITE_END
diff --git a/tests/parser/mailboxListTest.cpp b/tests/parser/mailboxListTest.cpp
index 7505acd9..2036863d 100644
--- a/tests/parser/mailboxListTest.cpp
+++ b/tests/parser/mailboxListTest.cpp
@@ -28,6 +28,7 @@ VMIME_TEST_SUITE_BEGIN(mailboxListTest)
 
 	VMIME_TEST_LIST_BEGIN
 		VMIME_TEST(testParseGroup)
+		VMIME_TEST(testBrokenGroup)
 	VMIME_TEST_LIST_END
 
 
@@ -44,4 +45,18 @@ VMIME_TEST_SUITE_BEGIN(mailboxListTest)
 		VASSERT_EQ("email", "[email protected]", mboxList.getMailboxAt(2)->getEmail().generate());
 	}
 
+	void testBrokenGroup() {
+
+		std::string bad(":,");
+
+		for (int i = 0 ; i < 10 ; ++i) {
+			bad = bad + bad;
+		}
+
+		vmime::mailboxList mboxList;
+		mboxList.parse(bad);
+
+		VASSERT_EQ("count", 0, mboxList.getMailboxCount());
+	}
+
 VMIME_TEST_SUITE_END