1465241 - Use of uninitialized pointers in ReadDBSubjectEntry()

Status: RESOLVED FIXED

(NSS :: Libraries, enhancement)

Description

[mozillabugs]

In an error case, ReadDBSubjectEntry() (security\nss\lib\softoken\legacydb\pcertdb.c) destroys an uninitialized object, causing the use of uninitialized pointers.

The bug is that line 2586, which initializes |tmpArena|, appears after the first |goto loser| statement (line 2583). If there is an OOM on line 2580, line 2583 is executed, which causes line 2616 to be executed, which attempts to destroy the "arena" described by the uninitialized object |tmpArena|. This causes the use of uninitialized pointers inside |tmpArena.arena|:

2568: static certDBEntrySubject *
2569: ReadDBSubjectEntry(NSSLOWCERTCertDBHandle *handle, SECItem *derSubject)
2570: {
2571:     /* |arena| isn't function-bounded, so cannot be a PORTCheapArenaPool. */
2572:     PLArenaPool *arena = NULL;
2573:     PORTCheapArenaPool tmpArena;
2574: 
2575:     certDBEntrySubject *entry;
2576:     SECItem dbkey;
2577:     SECItem dbentry;
2578:     SECStatus rv;
2579: 
2580:     arena = PORT_NewArena(DER_DEFAULT_CHUNKSIZE);
2581:     if (arena == NULL) {
2582:         PORT_SetError(SEC_ERROR_NO_MEMORY);
2583:         goto loser;
2584:     }
2585: 
2586:     PORT_InitCheapArena(&tmpArena, DER_DEFAULT_CHUNKSIZE);
...
2615: loser:
2616:     PORT_DestroyCheapArena(&tmpArena);
2617:     if (arena) {
2618:         PORT_FreeArena(arena, PR_FALSE);
2619:     }
2620: 
2621:     return (NULL);
2622: }

I will attempt a POC.

Comment 1

[Daniel Veditz [:dveditz]]

Does anyone even use the legacy DB code anymore? Maybe for profile migration (which would presumably read entries) so this might still be a relevant Firefox bug.

Comment 2

[Franziskus Kiefer [:franziskus]]

The legacy DB shouldn't be used anymore unless requested explicitly (which no one should do as this is certainly not the only bug).
Firefox upgrades from old versions or ESR52 still use it though.

Comment 3

[Phabricator Automation]

Attachment: Bug 1465241 - always init tmpArena

Comment 4

[Phabricator Automation]

Comment on attachment 8983332 [details]
Bug 1465241 - always init tmpArena

Martin Thomson [:mt:] has approved the revision.

https://phabricator.services.mozilla.com/D1552

Comment 5

[Franziskus Kiefer [:franziskus]]

https://hg.mozilla.org/projects/nss/rev/798e88281cef81baeb6a445aa8288b2fc752a30e