Closed Bug 1465241 Opened 2 years ago Closed 2 years ago

Use of uninitialized pointers in ReadDBSubjectEntry()

Categories

(NSS :: Libraries, enhancement)

3.34.1
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: mozillabugs, Assigned: franziskus, NeedInfo)

Details

(Keywords: csectype-uninitialized, sec-low)

Attachments

(1 file)

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.
Flags: needinfo?(mozillabugs)
Flags: sec-bounty?
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.
Flags: needinfo?(franziskuskiefer)
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Flags: needinfo?(franziskuskiefer)
Assignee: nobody → franziskuskiefer
Comment on attachment 8983332 [details]
Bug 1465241 - always init tmpArena

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

https://phabricator.services.mozilla.com/D1552
Attachment #8983332 - Flags: review+
https://hg.mozilla.org/projects/nss/rev/798e88281cef81baeb6a445aa8288b2fc752a30e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.38
Group: crypto-core-security → core-security-release
Flags: sec-bounty? → sec-bounty-
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.