Open Bug 1280066 Opened 4 years ago Updated 4 years ago

(coverity) resource leak: mailnews/mime/src/mimecms.cpp: a memory area returned by | mime_part_address(walker)| is not freed.

Categories

(MailNews Core :: MIME, defect)

defect
Not set

Tracking

(Not tracked)

People

(Reporter: ishikawa, Unassigned)

References

(Blocks 1 open bug, )

Details

(Keywords: coverity, Whiteboard: CID 450579)

Coverity found this:
 |mime_part_address(walker)| returns a memory area but
it is not freed (it is used in an if-expression and the value is forgotten there: thus the area is not freed.)

364    for (MimeObject *walker = obj; walker; walker = walker->parent) {
   4. Condition aAlreadyFoundTop, taking false branch
   11. Condition aAlreadyFoundTop, taking false branch
365      if (aAlreadyFoundTop) {
366        if (!mime_typep(walker, (MimeObjectClass *) &mimeEncryptedClass)
367            && !mime_typep(walker, (MimeObjectClass *) &mimeMultipartSignedClass)) {
368          ++aTopMessageNestLevel;
369        }
370      }
   5. Condition !aAlreadyFoundTop, taking true branch
   6. Condition !strcmp(mime_part_address(walker), walker->options->part_to_load), taking false branch
   12. Condition !aAlreadyFoundTop, taking true branch
   13. alloc_fn: Storage is returned from allocation function mime_part_address. [show details]
   14. noescape: Resource mime_part_address(walker) is not freed or pointed-to in strcmp.
   15. Condition !strcmp(mime_part_address(walker), walker->options->part_to_load), taking true branch
   CID 450579 (#1 of 1): Resource leak (RESOURCE_LEAK)16. leaked_storage: Failing to save or free storage allocated by mime_part_address(walker) leaks it.
371      if (!aAlreadyFoundTop && !strcmp(mime_part_address(walker), walker->options->part_to_load)) {
372        aAlreadyFoundTop = true;
373        aTopShownObject = walker;
374      }
   7. Condition !aAlreadyFoundTop, taking true branch
   8. Condition !walker->parent, taking false branch
375      if (!aAlreadyFoundTop && !walker->parent) {
376        // The mime part part_to_load is not a parent of the
377        // the crypto mime part passed in to this function as parameter obj.
378        // That means the crypto part belongs to another branch of the mime tree.
379        return -1;
380      }
   9. Jumping back to the beginning of the loop
381    }
382  }
383

Observation: 
|mime_part_address(walker)| returns 
case 1 - an area returned by strdup("0"), strdup(buf) or
case 2 - an area obtained by PR_MALLOC()

I am not sure if we can release the memory area returned in case 1  by PR_FREEIF().
So I would suggest that, for case 1, we do the following instead of calling strdup(buf), so that we can call PR_FREEIF().
 
char *p = PR_MALLOC(strlen(buf) + 1); 
if (p) {
  strcpy(p, buf);
  return p;
} else 
{
  return 0; /* MIME_OUT_OF_MEMORY */
}
You need to log in before you can comment on or make changes to this bug.