Open Bug 1285262 Opened 8 years ago Updated 2 years ago

(coverity) resource leak: mailnews/imap/src/nsImapServerResponseParser.cpp: |mimeHeaderData| going out of scope leaks the storage it points to.

Categories

(MailNews Core :: Networking: IMAP, defect)

defect

Tracking

(Not tracked)

People

(Reporter: ishikawa, Unassigned)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity, memory-leak, Whiteboard: CID 450529)

Coverity found this: Area pointed by |mimeHeaderData| is not freed always. (It is allocated by PR_Malloc()). 2623// mime_header_data should not be streamed out; rather, it should be 2624// buffered in the nsIMAPBodyShell. 2625// This is because we are still in the process of generating enough 2626// information from the server (such as the MIME header's size) so that Test Coverage Test Policy Rule Exclusions Line Impact Function Impact 2627// we can construct the final output stream. 2628void nsImapServerResponseParser::mime_header_data() 2629{ 2630 char *partNumber = PL_strdup(fNextToken); 1. Condition partNumber, taking true branch 2631 if (partNumber) 2632 { 2633 char *start = partNumber + 5, *end = partNumber + 5; // 5 == strlen("BODY[") 2. Condition this->ContinueParse(), taking true branch 3. Condition end, taking true branch 4. Condition *end != 'M', taking true branch 5. Condition *end != 'm', taking true branch 7. Condition this->ContinueParse(), taking true branch 8. Condition end, taking true branch 9. Condition *end != 'M', taking true branch 10. Condition *end != 'm', taking false branch 2634 while (ContinueParse() && end && *end != 'M' && *end != 'm') 2635 { 2636 end++; 6. Jumping back to the beginning of the loop 2637 } 11. Condition end, taking true branch 12. Condition *end == 'M', taking false branch 13. Condition *end == 'm', taking true branch 2638 if (end && (*end == 'M' || *end == 'm')) 2639 { 2640 *(end-1) = 0; 2641 AdvanceToNextToken(); 14. alloc_fn: Storage is returned from allocation function CreateAstring. [show details] 15. var_assign: Assigning: mimeHeaderData = storage returned from this->CreateAstring(). 2642 char *mimeHeaderData = CreateAstring(); // is it really this simple? 2643 AdvanceToNextToken(); 16. Condition this->m_shell.operator bool(), taking false branch 2644 if (m_shell) 2645 { 2646 m_shell->AdoptMimeHeader(start, mimeHeaderData); 2647 } CID 450529 (#1 of 1): Resource leak (RESOURCE_LEAK)17. leaked_storage: Variable mimeHeaderData going out of scope leaks the storage it points to. 2648 } 2649 else 2650 { 2651 SetSyntaxError(true); 2652 } 2653 PR_Free(partNumber); // partNumber is not adopted by the body shell. 2654 } 2655 else 2656 { 2657 HandleMemoryFailure(); 2658 } 2659} 2660 Observation: Since |mimeHeaderData| is allocated an area by PR_Malloc(), PR_Free() should release the area. this-> CreateAstring() ===> CreateLiteral() ===> PR_Malloc().
The following lines were misquoted... Test Coverage Test Policy Rule Exclusions Line Impact Function Impact
How much are we leaking?
Keywords: mlk
It is a variable: It seems something like this: The scanner is looking at something like this : "{number" The pointer is at "{". The number is scanned to produce the size of the PR_Malloced area. That is what is lost in |mimeHeaderData|. Caller sequence: mimeHeaderData receives a pointer from CreateAstring(). 208char *nsIMAPGenericParser::CreateAstring() 209{ 1. Condition *this->fNextToken == '{', taking true branch 210 if (*fNextToken == '{') 2. alloc_fn: Storage is returned from allocation function CreateLiteral. [show details] 3. return_alloc_fn: Directly returning storage allocated by CreateLiteral. 211 return CreateLiteral(); // literal 212 else if (*fNextToken == '"') 213 return CreateQuoted(); // quoted 214 else 215 return CreateAtom(true); // atom 216} 217 CreateAstring() returns a pointer from C reateLilteral(): 351char *nsIMAPGenericParser::CreateLiteral() 352{ 353 int32_t numberOfCharsInMessage = atoi(fNextToken + 1); 354 uint32_t numBytes = numberOfCharsInMessage + 1; 1. Condition !numBytes, taking false branch 355 NS_ASSERTION(numBytes, "overflow!"); 2. Condition !numBytes, taking false branch 356 if (!numBytes) 357 return nullptr; 3. alloc_fn: Storage is returned from allocation function PR_Malloc. 4. var_assign: Assigning: returnString = PR_Malloc(numBytes). 358 char *returnString = (char *)PR_Malloc(numBytes); 5. Condition !returnString, taking false branch 359 if (!returnString) 360 { 361 HandleMemoryFailure(); 362 return nullptr; 363 } 364
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.