1771497 - Uninitialized value in cert_VerifyCertChainOld

Status: RESOLVED FIXED

(NSS :: Tools, defect, P3)

Description

[Nico]

Attachment: Docker files to reproduce the bug

Steps to reproduce:

During processing of the attached certificate for vfychain via:
'''
/dist/Debug/bin/vfychain -a /testcase
'''
an conditional jump or move which depends on uninitialised value is triggered.

For reproduction of the crash, I attach a Docker image. Run ./build_upstream.sh to build the docker image and ./reproduce-upstream.sh to reproduce the bug.

Since I have no experience with these bugs in cryptographic software, I set the security flag.

Actual results:

Valgrind Output:

==1== Memcheck, a memory error detector
==1== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==1== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==1== Command: /dist/Debug/bin/vfychain -a /testcase
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x486F437: cert_VerifyCertChainOld (certvfy.c:796)
==1== by 0x486FC97: cert_VerifyCertChain (certvfy.c:1001)
==1== by 0x4870A1C: CERT_VerifyCertificate (certvfy.c:1496)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a stack allocation
==1== at 0x486EDD1: cert_VerifyCertChainOld (certvfy.c:598)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x486F445: cert_VerifyCertChainOld (certvfy.c:796)
==1== by 0x486FC97: cert_VerifyCertChain (certvfy.c:1001)
==1== by 0x4870A1C: CERT_VerifyCertificate (certvfy.c:1496)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a stack allocation
==1== at 0x486EDD1: cert_VerifyCertChainOld (certvfy.c:598)
==1==
Chain is bad!
PROBLEM WITH THE CERT CHAIN:
CERT 0. CN=ABC :
ERROR -8184: Improperly formatted time string.
ERROR -8182: Peer's certificate has an invalid signature.
ERROR -8162: The certificate issuer's certificate has expired. Check your system date and time.
CN=ABC
CERT 1. CN=ABC [Certificate Authority]:
ERROR -8156: Issuer certificate is invalid.
ERROR -8172: Peer's certificate issuer has been marked as not trusted by the user.
CN=ABC
==1==
==1== HEAP SUMMARY:
==1== in use at exit: 1,350 bytes in 7 blocks
==1== total heap usage: 1,144 allocs, 1,137 frees, 615,256 bytes allocated
==1==
==1== LEAK SUMMARY:
==1== definitely lost: 0 bytes in 0 blocks
==1== indirectly lost: 0 bytes in 0 blocks
==1== possibly lost: 0 bytes in 0 blocks
==1== still reachable: 1,350 bytes in 7 blocks
==1== suppressed: 0 bytes in 0 blocks
==1== Rerun with --leak-check=full to see details of leaked memory
==1==
==1== For lists of detected and suppressed errors, rerun with: -s
==1== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Expected results:

Expected the correct exception handling of invalid files / certificates.

Comment 1

[Nico]

changeset: 21e7aaa1f7d94bca15d997e5b4c2329b32fad21a

Comment 2

[John Schanck [:jschanck]]

Thanks again.

This happens in an error handling path for end-entity certificates that have a BasicConstraints extension. The CERT_DecodeBasicConstraintValue fails to initialize the pathLenConstraint field of the returned CERTBasicConstraints for end-entity certificates. This could cause cert_VerifyCertChainOld to set the SEC_ERROR_PATH_LEN_CONSTRAINT_INVALID instead of the expected SEC_ERROR_CA_CERT_INVALID, but it will not change the fact that an error is returned.

Comment 3

[John Schanck [:jschanck]]

Attachment: Bug 1771497 - Uninitialized value in cert_VerifyCertChainOld. r=#nss-reviewers

Comment 4

[Daniel Veditz [:dveditz]]

Returning the wrong error doesn't sound like a security issue that needs to be hidden, is it?

Comment 5

[John Schanck [:jschanck]]

I agree, this doesn't need to be hidden.

Comment 6

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/23be110cfe8ad25c9939e71f226e523304d61a19