459359 - ForwardBuilderState object is leaked when AIA path incorrect

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Slavomir Katuscak]

Vfychain crashes and dumps core when AIA path doesn't exist.

Steps to reproduce:

1. Generate root CA cert.
certutil -N -d RootDB -f RootDB/dbpasswd
certutil -s "CN=Root ROOT CA, O=Root, C=US" -S -n Root -t CTu,CTu,CTu -v 600 -x -d RootDB -1 -2 -5 -f RootDB/dbpasswd -z /export/tinderlight/data/chains/mozilla/tests_results/security/ciotat.12/tests_noise.364

2. Generate intermediate CA1 signed by root CA.
certutil -N -d CA1DB -f CA1DB/dbpasswd
certutil -s "CN=CA1 Intermediate, O=CA1, C=US" -R -2 -d CA1DB -f CA1DB/dbpasswd -z /export/tinderlight/data/chains/mozilla/tests_results/security/ciotat.12/tests_noise.364 -o CA1Req.der
certutil -C -c Root -v 60 -d RootDB -i CA1Req.der -o CA1Root.der -f RootDB/dbpasswd  
certutil -A -n CA1 -t u,u,u -d CA1DB -f CA1DB/dbpasswd -i CA1Root.der

3. Generate intermediate CA2 signed by CA1, with AIA extension.
certutil -N -d CA2DB -f CA2DB/dbpasswd
certutil -s "CN=CA2 Intermediate, O=CA2, C=US" -R -2 -d CA2DB -f CA2DB/dbpasswd -z /export/tinderlight/data/chains/mozilla/tests_results/security/ciotat.12/tests_noise.364 -o CA2Req.der
certutil -C -c CA1 -v 60 -d CA1DB -i CA2Req.der -o CA2CA1.der -f CA1DB/dbpasswd   --extAIA
certutil -A -n CA2 -t u,u,u -d CA2DB -f CA2DB/dbpasswd -i CA2CA1.der

When asked for AIA details, set CA issuers (1), then uniformResourceidentifier
(7) and then INCORRECT http path to CA1 cert. 

4. Generate user certificate signed by CA2.
certutil -N -d UserDB -f UserDB/dbpasswd
certutil -s "CN=User EE, O=User, C=US" -R  -d UserDB -f UserDB/dbpasswd -z /export/tinderlight/data/chains/mozilla/tests_results/security/ciotat.12/tests_noise.364 -o UserReq.der
certutil -C -c CA2 -v 60 -d CA2DB -i UserReq.der -o UserCA2.der -f CA2DB/dbpasswd  
certutil -A -n User -t u,u,u -d UserDB -f UserDB/dbpasswd -i UserCA2.der

5. Verify user cert trusting Root CA with fetch flag set (intermediate CA1 not
set on command line, should be fetched).
vfychain -d UserDB -pp -vv -f   UserCA2.der CA2CA1.der  -t Root.der

Output is:
Chain is bad, -8179 = Peer's Certificate issuer is not recognized.
1[42418]: Class BigInt leaked 1 objects of size 8 bytes, total = 8 bytes
1[42418]: Class Cert leaked 3 objects of size 136 bytes, total = 408 bytes
1[42418]: Class CertBasicConstraints leaked 1 objects of size 8 bytes, total = 8 bytes
1[42418]: Class CertSelector leaked 1 objects of size 12 bytes, total = 12 bytes
1[42418]: Class CertStore leaked 1 objects of size 32 bytes, total = 32 bytes
1[42418]: Class ComCertSelParams leaked 1 objects of size 76 bytes, total = 76 bytes
1[42418]: Class Date leaked 2 objects of size 12 bytes, total = 24 bytes
1[42418]: Class Error leaked 1 objects of size 20 bytes, total = 20 bytes
1[42418]: Class ForwardBuilderState leaked 1 objects of size 216 bytes, total = 216 bytes
1[42418]: Class GeneralName leaked 2 objects of size 24 bytes, total = 48 bytes
1[42418]: Class InfoAccess leaked 1 objects of size 8 bytes, total = 8 bytes
1[42418]: Class List leaked 10 objects of size 20 bytes, total = 200 bytes
1[42418]: Class PublicKey leaked 2 objects of size 4 bytes, total = 8 bytes
1[42418]: Class VerifyNode leaked 2 objects of size 16 bytes, total = 32 bytes
1[42418]: Class X500Name leaked 5 objects of size 24 bytes, total = 120 bytes
1[42418]: Assertion failure: numLeakedObjects == 0, at pkix_pl_lifecycle.c:294
Assertion failure: numLeakedObjects == 0, at pkix_pl_lifecycle.c:294
./all.sh: line 57:   897 Abort                   (core dumped) ${BINDIR}/vfychain ${DB_OPT} -pp -vv ${FETCH_OPT} ${POLICY_OPT} ${VFY_CERTS} ${TRUST_OPT}

6. Look into the core file.
dbx /export/tinderlight/data/chains/mozilla/dist/SunOS5.9_DBG.OBJ/bin/vfychain core

(dbx) where
current thread: t@1
  [1] __lwp_kill(0x0, 0x6, 0x0, 0xfee3c000, 0x0, 0x0), at 0xfee1f684 
  [2] raise(0x6, 0x0, 0xffbfeeb8, 0xfee3c000, 0x4, 0x4), at 0xfedd0884 
  [3] abort(0x45, 0xfefe6110, 0xff265058, 0xff265070, 0x126, 0x5a811), at 0xfedb6cd0 
=>[4] PR_Assert(s = 0xff265058 "numLeakedObjects == 0", file = 0xff265070 "pkix_pl_lifecycle.c", ln = 294), line 550 in "prlog.c"
  [5] PKIX_PL_Shutdown(plContext = 0x561b0), line 294 in "pkix_pl_lifecycle.c"
  [6] PKIX_Shutdown(plContext = 0x561b0), line 231 in "pkix_lifecycle.c"
  [7] NSS_Shutdown(), line 884 in "nssinit.c"
  [8] main(argc = 10, argv = 0xffbff36c, envp = 0xffbff398), line 565 in "vfychain.c"

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Slavo, since 3.12.1 has been released, the target milestone for all new 
trunk bugs should be 3.12.2 (until we release that version).

Comment 2

[Alexei Volkov]

Attachment: Patch v1 fix leak in case of failure to fetch cert through AIA cert ext

Do not unconditionally goto cleanup in case of AIA mgr failure. We should check the error and save it in the log.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 344035 [details] [diff] [review]
Patch v1 fix leak in case of failure to fetch cert through AIA cert ext

r+.  I want to ask that some explanatory comments be added.
Based on a superficial reading, the following lines:

>+                    if (state->verifyNode != NULL) {
>+                        PKIX_CHECK_FATAL(pkix_VerifyNode_Create
>+                                         (state->prevCert,
>+                                          0, NULL,
>+                                          &verifyNode,
>+                                          plContext),

appear to say "if a verify node exists, create one".
That doesn't seem to make sense, until one learns that 
state->verifyNode is actually the root node of a tree, and 
the local variable verifyNode is a new child node to be 
added to that tree.  The comments need to explain that.

Comment 4

[Alexei Volkov]

Patch is integrated with suggested comments.