From 561746081f633245b326e31e6ef0f2ef20b48ef6 Mon Sep 17 00:00:00 2001 From: vincent-richard Date: Tue, 25 Jan 2022 10:28:20 +0100 Subject: Fixed possible recursion crash when parsing mailbox groups. --- src/vmime/address.cpp | 42 ++++++++++++++++++++++++++++++--------- src/vmime/address.hpp | 1 + src/vmime/addressList.cpp | 2 +- src/vmime/mailboxField.cpp | 2 +- src/vmime/mailboxGroup.cpp | 10 ++-------- src/vmime/mailboxList.cpp | 2 +- tests/parser/mailboxGroupTest.cpp | 28 ++++++++++++++++++++++++++ tests/parser/mailboxListTest.cpp | 15 ++++++++++++++ 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::parseNext( const size_t position, const size_t end, size_t* newPosition, + const bool allowGroup, bool *isLastAddressOfGroup ) { @@ -203,13 +204,16 @@ shared_ptr
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::parseNext( shared_ptr
parsedAddress; if (isGroup) { - parsedAddress = make_shared (); + + if (allowGroup) { + + parsedAddress = make_shared (); + + } 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 (); } - 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
parsedAddress = address::parseNext(ctx, buffer, pos, end, &pos, NULL); + shared_ptr
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
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
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 group = dynamicCast (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
parsedAddress = address::parseNext(ctx, buffer, pos, end, &pos, NULL); + shared_ptr
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:mbox1@domain.com,group2:mbox2@domain.com;,mbox3@domain.com;"); + + VASSERT_EQ("name", "group1", mgrp.getName().getWholeBuffer()); + VASSERT_EQ("count", 2, mgrp.getMailboxCount()); + VASSERT_EQ("mbox1", "mbox1@domain.com", mgrp.getMailboxAt(0)->getEmail()); + VASSERT_EQ("mbox2", "mbox3@domain.com", 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", "email3@domain3.com", 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 -- cgit v1.2.3