Coverity 297 memory leak in sec_asn1d_push_state

RESOLVED INVALID

Status

P2
normal
RESOLVED INVALID
12 years ago
12 years ago

People

(Reporter: nelson, Assigned: alvolkov.bgs)

Tracking

({coverity, memory-leak})

3.11.1
3.11.3
coverity, memory-leak

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: CID 297)

In sec_asn1d_push_state, the code allocates "new_state".
It may be allocated from an arenapool, or from the heap, depending on 
whether cx->our_pool is NULL or not.

If our_pool is NULL, then new_state is leaked at label "loser".

Also, in two places in this routine, the code tests for "state != NULL"
but does not test cx->our_pool != NULL.  Consequently, it may pass a 
NULL our_pool value to PORT_ArenaMark and PORT_ArenaRelease.
(Reporter)

Updated

12 years ago
Priority: -- → P2
Whiteboard: CID 297
Keywords: mlk
(Assignee)

Comment 1

12 years ago
SEC_ASN1DecoderContext structure is initialized in SEC_ASN1DecoderStart. 

    our_pool = PORT_NewArena (SEC_ASN1_DEFAULT_ARENA_SIZE);
    if (our_pool == NULL)
	return NULL;

    cx = (SEC_ASN1DecoderContext*)PORT_ArenaZAlloc (our_pool, sizeof(*cx));
    if (cx == NULL) {
	PORT_FreeArena (our_pool, PR_FALSE);
	return NULL;
    }

    cx->our_pool = our_pool;

I don't see any way how cx->our_pool would be set to NULL unless SEC_ASN1DecoderContext is created not by SEC_ASN1DecoderStart.

ASN1D leaks memory in case when their_pool is null and if there was a decoding error, as we don't have proper mechanism to free memory allocated for already decoded objects.

See bug 95311
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.