Fixed possible recursion crash when parsing mailbox groups.

This commit is contained in:
vincent-richard 2022-01-25 10:28:20 +01:00
parent 23ab2a6a3c
commit 561746081f
8 changed files with 82 additions and 20 deletions

View File

@ -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;
}

View File

@ -78,6 +78,7 @@ public:
const size_t position,
const size_t end,
size_t* newPosition,
const bool allowGroup,
bool *isLastAddressOfGroup
);
};

View File

@ -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);

View File

@ -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) {

View File

@ -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 {

View File

@ -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) {

View File

@ -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

View File

@ -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