337326 - Coverity Leak in nsslowcert_UpdateSubjectEmailAddr (security/nss/lib/softoken/pcertdb.c)

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Kenneth Herron]

Please refer to the sample URL. At line 2699, |nsslowcert_FixupEmailAddr| allocates a copy of |emailAddr| and returns a pointer to it, which is stored in |emailAddr|. At lines 2723 and 2733, the function can return without freeing this copy.

Comment 1

[Alexei Volkov]

Attachment: Fix for the problem

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 222285 [details] [diff] [review]
Fix for the problem

There's one problem with this patch.

In the original code, the interpretation of "index" was different depending
on the "updateType".  If updateType was nsslowcert_remove, then index == -1
meant a shortcut to success (don't have to do anything to remove the entry
if it's not actually there).  But if updateType was any other value besides
nsslowcert_remove, then the shortcut to success is index != 1, the opposite
of the other test.  

This patch substitutes (index == -1) as the shortcut for all updateTypes,
which I believe causes all updateTypes except nsslowcert_remove to fail.
So, r- for that.   Otherwise, this patch was OK.

Comment 3

[Alexei Volkov]

Attachment: prev patch, but with restored "if"s

missread the code.

Comment 4

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 222386 [details] [diff] [review]
prev patch, but with restored "if"s

r=nelson

Comment 5

[Alexei Volkov]

trunk:
/cvsroot/mozilla/security/nss/lib/softoken/pcertdb.c,v  <--  pcertdb.c
new revision: 1.63; previous revision: 1.62

3.11 branch:
/cvsroot/mozilla/security/nss/lib/softoken/pcertdb.c,v  <--  pcertdb.c
new revision: 1.53.2.10; previous revision: 1.53.2.9

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

CID 581