337495 - Coverity Double free in CRMF_CertReqMsgSetSignaturePOP (security/nss/lib/crmf/crmfpop.c)

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Kenneth Herron]

This is coverity CID 944. Please refer to the sample URL. At line 339, |derDest.data| is freed. If the test on line 340 succeeds, execution branches to line 346 where |derDest.data| is freed again a few lines later.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Yup, it's a double-free allright.  

Comment 2

[Alexei Volkov]

Attachment: fix

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 222750 [details] [diff] [review]
fix

This code could surely use some comments, and some variables should be 
renamed.  For example, derDest should be derTemp, because it is NOT the
destination, but only a temporary holder.  

But this patch appears to correctly fix the bug it targets.
r=nelson

Comment 4

[Alexei Volkov]

Bug fix integration:
trunk:
/cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v  <--  crmfpop.c
new revision: 1.5; previous revision: 1.4

3.11 branch:
/cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v  <--  crmfpop.c
new revision: 1.3.28.2; previous revision: 1.3.28.1

Comment 5

[Alexei Volkov]

Attachment: variable rename

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 222949 [details] [diff] [review]
variable rename

r=nelson

Comment 7

[Alexei Volkov]

second patch integration:
trunk:
/cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v  <--  crmfpop.c
new revision: 1.6; previous revision: 1.5

3.11 branch:
/cvsroot/mozilla/security/nss/lib/crmf/crmfpop.c,v  <--  crmfpop.c
new revision: 1.3.28.3; previous revision: 1.3.28.2

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

CID 944