From ee0286673015d4facc08e4cbd35392500e345f55 Mon Sep 17 00:00:00 2001 From: Alex J <245064032+router0mail@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:14:16 +0200 Subject: [PATCH] fix(mail): check buffer length before reading TNEF properties Several places in the TNEF attachment decoder read a fixed number of bytes, or index at a fixed offset, out of a MAPI property's data buffer without checking the buffer is actually that large: - the possibly-embedded-TNEF signature, read at offset 16 for an object property and at offset 0 for plain file data - the same object-attachment data, when building the actual attachment content a few lines later: the length used there is derived from the same unchecked offset and underflows to a huge value for a short buffer - the classification flag, read as an 8-byte value - the recurrence pattern, indexed up to offset 0x23 but only checked for 0x1F bytes A short enough attachment property made these read, or in the attachment case allocate, past the end of the buffer. Add the missing length checks; where no further data is left to build from, skip that part of the output instead. --- SoObjects/Mailer/SOGoTNEFMailBodyPart.m | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/SoObjects/Mailer/SOGoTNEFMailBodyPart.m b/SoObjects/Mailer/SOGoTNEFMailBodyPart.m index 2f658aa0f..f306d0646 100644 --- a/SoObjects/Mailer/SOGoTNEFMailBodyPart.m +++ b/SoObjects/Mailer/SOGoTNEFMailBodyPart.m @@ -586,7 +586,7 @@ unsigned char GetRruleMonthNum(unsigned char a, unsigned char b) { // Classification filedata = MAPIFindUserProp(&(tnef.MapiProperties), PROP_TAG(PT_BOOLEAN, 0x8506)); - if (filedata != MAPI_UNDEFINED) + if (filedata != MAPI_UNDEFINED && filedata->size >= (int)sizeof(DDWORD)) { classification = (DDWORD *)filedata->data; if (*classification == 1) @@ -597,7 +597,7 @@ unsigned char GetRruleMonthNum(unsigned char a, unsigned char b) { // Repeating rule filedata = MAPIFindUserProp(&(tnef.MapiProperties), PROP_TAG(PT_BINARY, 0x8216)); - if (filedata != MAPI_UNDEFINED && filedata->size >= 0x1F) + if (filedata != MAPI_UNDEFINED && filedata->size >= 0x24) { NSMutableString *rrule = [NSMutableString string]; unsigned char *recurData = filedata->data; @@ -712,7 +712,7 @@ unsigned char GetRruleMonthNum(unsigned char a, unsigned char b) { if (isObject) { // This is an "embedded object", so skip the 16-byte identifier first. - if (NULL != filedata->data) { + if (NULL != filedata->data && filedata->size >= 16 + (int)sizeof(DWORD)) { memcpy(&object_signature, filedata->data + 16, sizeof(DWORD)); if (TNEFCheckForSignature(object_signature) == 0) { TNEFInitialize(&emb_tnef); @@ -725,7 +725,7 @@ unsigned char GetRruleMonthNum(unsigned char a, unsigned char b) { } } } - else + else if (NULL != filedata->data && filedata->size >= (int)sizeof(DWORD)) { memcpy(&object_signature, filedata->data, sizeof(DWORD)); if (TNEFCheckForSignature(object_signature) == 0) { @@ -817,19 +817,23 @@ unsigned char GetRruleMonthNum(unsigned char a, unsigned char b) { cid = [NSString stringWithUTF8String: (const char *)prop->data]; } - NSData *attachment; + NSData *attachment = nil; if (NULL != filedata->data) { if (isObject) - attachment = [NSData dataWithBytes: filedata->data + 16 length: filedata->size - 16]; + { + if (filedata->size >= 16) + attachment = [NSData dataWithBytes: filedata->data + 16 length: filedata->size - 16]; + } else attachment = [NSData dataWithBytes: filedata->data length: filedata->size]; + } + if (attachment) [self bodyPartForAttachment: attachment withName: partName andType: type andSubtype: subtype andContentId: cid]; - } // count++; }