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)
MailNews Core
Networking: IMAP
Tracking
(Not tracked)
NEW
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().
Reporter | ||
Comment 1•8 years ago
|
||
The following lines were misquoted...
Test Coverage
Test Policy Rule
Exclusions
Line Impact
Function Impact
Reporter | ||
Comment 3•8 years ago
|
||
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
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.
Description
•