Closed Bug 1771498 Opened 2 years ago Closed 2 years ago

Uninitialized value in cert_ComputeCertType

Categories

(NSS :: Tools, defect, P3)

Product:
NSS
Component:
Tools
Version:
3.79
Type:
defect

Tracking

(firefox-esr91 wontfix, firefox-esr102104+ fixed, firefox101 wontfix, firefox102 wontfix, firefox103 fixed)

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
firefox-esr91 --- wontfix
firefox-esr102 104+ fixed
firefox101 --- wontfix
firefox102 --- wontfix
firefox103 --- fixed

People

(Reporter: nico.schiller, Assigned: jschanck)

References

Details

(Keywords: csectype-uninitialized, sec-low, Whiteboard: [nss-nofx][post-critsmash-triage][adv-main103-][adv-esr102.2-])

Attachments

(2 files)

Docker files to reproduce the bug
3.19 KB, application/zip
Details
Bug 1771498 - Uninitialized value in cert_ComputeCertType. r=djackson
48 bytes, text/x-phabricator-request
Details | Review

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 0x48CD839: cert_ComputeCertType (certdb.c:523)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1== by 0x10E953: getCert (vfychain.c:198)
==1== by 0x10F8BD: main (vfychain.c:608)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CD86E: cert_ComputeCertType (certdb.c:531)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1== by 0x10E953: getCert (vfychain.c:198)
==1== by 0x10F8BD: main (vfychain.c:608)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CDA16: cert_ComputeCertType (certdb.c:593)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1== by 0x10E953: getCert (vfychain.c:198)
==1== by 0x10F8BD: main (vfychain.c:608)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CDA29: cert_ComputeCertType (certdb.c:596)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1== by 0x10E953: getCert (vfychain.c:198)
==1== by 0x10F8BD: main (vfychain.c:608)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CD630: cert_GetCertType (certdb.c:437)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x48CE027: CERT_DecodeDERCertificate (certdb.c:811)
==1== by 0x48E1D81: nssDecodedPKIXCertificate_Create (pki3hack.c:496)
==1== by 0x48E2CC6: stan_GetCERTCertificate (pki3hack.c:900)
==1== by 0x48E2F7C: STAN_GetCERTCertificate (pki3hack.c:978)
==1== by 0x48DCEF4: CERT_NewTempCertificate (stanpcertdb.c:417)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CD839: cert_ComputeCertType (certdb.c:523)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CD86E: cert_ComputeCertType (certdb.c:531)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CDA16: cert_ComputeCertType (certdb.c:593)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x48CDA29: cert_ComputeCertType (certdb.c:596)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1==
==1== Conditional jump or move depends on uninitialised value(s)
==1== at 0x487084C: CERT_VerifyCertificate (certvfy.c:1473)
==1== by 0x10FA19: main (vfychain.c:638)
==1== Uninitialised value was created by a heap allocation
==1== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==1== by 0x4C7B23C: PR_Malloc (prmem.c:448)
==1== by 0x494089C: PORT_Alloc_Util (secport.c:87)
==1== by 0x48D2B80: CERT_FindBitStringExtension (certxutl.c:420)
==1== by 0x48D1FB8: CERT_FindNSCertTypeExtension (certv3.c:51)
==1== by 0x48CD78F: cert_ComputeCertType (certdb.c:497)
==1== by 0x48CD644: cert_GetCertType (certdb.c:441)
==1== by 0x487060B: CERT_VerifyCertificate (certvfy.c:1419)
==1== by 0x10FA19: main (vfychain.c:638)
==1==
Chain is bad!
PROBLEM WITH THE CERT CHAIN:
CERT 0. CN=AB\ :
ERROR -8181: Peer's Certificate has expired.
ERROR -8101: Certificate type not approved for application.
Cert cannot be used for SSL.
==1==
==1== HEAP SUMMARY:
==1== in use at exit: 1,350 bytes in 7 blocks
==1== total heap usage: 1,110 allocs, 1,103 frees, 165,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: 10 errors from 10 contexts (suppressed: 0 from 0)

Expected results:

Expected the correct exception handling of invalid files / certificates.

changeset: 21e7aaa1f7d94bca15d997e5b4c2329b32fad21a

Thanks for this report.

The certificate has a Netscape cert-type extension (oid 2.16.840.1.113730.1.1) that contains an empty bitstring. After parsing this extension, the CERT_FindBitStringExtension function return a SECItem with a data field that points to a one byte allocation and with the length field set to 0.

There are several errors here.

  • CERT_FindBitStringExtension fails to initialize the one byte allocation,
  • CERT_FindBitStringExtension calls memcpy with a null source pointer with a 0 length parameter,
  • The caller cert_ComputeCertTypeExtension function fails to check the length parameter of the returned SECItem before accessing its first byte.

The memory leak appears to be a separate bug. But I'll include a fix in the patch.

Assignee: nobody → jschanck
Severity: -- → S4
Priority: -- → P3
Whiteboard: [nss-nofx]
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true

What security severity would you give this bug, John? I assume [nss-nofx] means this only affects NSS users who are not Firefox.

Flags: needinfo?(jschanck)

Right this doesn't affect Firefox. This would only be an issue if a CA signed a certificate with an empty keyUsage extension, which would be unusual. I don't see an obvious attack, but if an attack exists it probably involves a malicious CA. Let's go with sec-low.

Flags: needinfo?(jschanck)
Keywords: sec-low

https://hg.mozilla.org/projects/nss/rev/b28bc4cd74e6bcf8849952f3389d415a92fb5710

Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.80
Group: crypto-core-security → core-security-release
status-firefox101: --- → wontfix
status-firefox102: --- → wontfix
status-firefox103: --- → fixed
status-firefox-esr102: --- → fix-optional
status-firefox-esr91: --- → wontfix
Flags: qe-verify-
Whiteboard: [nss-nofx] → [nss-nofx][post-critsmash-triage]

(In reply to John Schanck [:jschanck] (PTO until Aug 8) from comment #5)

Right this doesn't affect Firefox. This would only be an issue if a CA signed a certificate with an empty keyUsage extension, which would be unusual. I don't see an obvious attack, but if an attack exists it probably involves a malicious CA. Let's go with sec-low.

So, my understanding is that it doesn't affect Firefox. Is this because the cert wouldn't be acceptable at all or because it's unlikely to be issued by a trusted CA?

Whiteboard: [nss-nofx][post-critsmash-triage] → [nss-nofx][post-critsmash-triage][adv-main103+]
Whiteboard: [nss-nofx][post-critsmash-triage][adv-main103+] → [nss-nofx][post-critsmash-triage][adv-main103-]
No longer blocks: 1780022
Whiteboard: [nss-nofx][post-critsmash-triage][adv-main103-] → [nss-nofx][post-critsmash-triage][adv-main103-][adv-esr102.2-]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: