Closed Bug 1460673 Opened 2 years ago Closed Last year

Crash in SEC_ASN1DecoderFinish_Util importing PKCS#12 file encrypted with AES

Categories

(NSS :: Libraries, defect, critical)

Unspecified
Linux
defect
Not set
critical

Tracking

(firefox-esr52 wontfix, firefox-esr60 fixed, firefox60 fixed, firefox61 fixed, firefox62 fixed)

RESOLVED FIXED
Tracking Status
firefox-esr52 --- wontfix
firefox-esr60 --- fixed
firefox60 --- fixed
firefox61 --- fixed
firefox62 --- fixed

People

(Reporter: keeler, Assigned: franziskus)

References

Details

(Keywords: crash, sec-moderate)

Crash Data

Attachments

(4 files)

Attached file import-me-to-crash.p12
This bug was filed from the Socorro interface and is
report bp-53c9dfd0-6a2f-4543-a702-f4d7e0180510.
=============================================================

Top 10 frames of crashing thread:

0 libnssutil3.so SEC_ASN1DecoderFinish_Util security/nss/lib/util/secasn1d.c:2980
1 libsmime3.so SEC_PKCS7DecoderUpdate security/nss/lib/pkcs7/p7decode.c:1048
2 libnssutil3.so SEC_ASN1DecoderUpdate_Util security/nss/lib/util/secasn1d.c:2926
3 libsmime3.so sec_pkcs12_decoder_asafes_callback security/nss/lib/pkcs12/p12d.c:853
4 libsmime3.so sec_pkcs7_decoder_filter security/nss/lib/pkcs7/p7decode.c:172
5 libnssutil3.so SEC_ASN1DecoderUpdate_Util security/nss/lib/util/secasn1d.c:2926
6 libsmime3.so SEC_PKCS7DecoderUpdate security/nss/lib/pkcs7/p7decode.c:1037
7 libsmime3.so sec_pkcs12_decode_asafes_cinfo_update security/nss/lib/pkcs12/p12d.c:955
8 libnssutil3.so SEC_ASN1DecoderUpdate_Util security/nss/lib/util/secasn1d.c:2926
9 libsmime3.so SEC_PKCS12DecoderUpdate security/nss/lib/pkcs12/p12d.c:1288

=============================================================


Discovered this from https://www.mail-archive.com/dev-tech-crypto@lists.mozilla.org/msg12911.html
This appears to be an NSS bug. I'll attach a reduced testcase shortly.
Assignee: nobody → nobody
Component: Security: PSM → Libraries
Product: Core → NSS
Version: unspecified → other
STR:
1. Compile libdislocator from https://github.com/mirrorer/afl/tree/master/libdislocator
2. Compile test.cpp (`make`)
3. run `LD_PRELOAD=/path/to/libdislocator.so ./test`
Thanks for filing David. I responded to Jonathan who reported the issue to let him know we're looking into it.
Running this with ASAN gives more information. The issue is that SEC_PKCS7DecoderFinish is called before decoding is done, i.e. sec_pkcs12_decoder_wrap_p7_update is called on the freed p12dcx->currentASafeP7Dcx producing a heap-use-after-free.

Making this sec-moderate. A user has to actively import a p12 file into Firefox or call pk12util on the command line to be affected.
Assignee: nobody → franziskuskiefer
Keywords: sec-moderate
The reason for crashing here is that the p7 processor wasn't cleared after finishing the first P7 bag (for a P12 that has two P7 bags).
This patch doesn't fix the import itself but only the uaf.
The decoder isn't supporting this properly. Firefox will show an unknown P12 import error now.

test
This fixes the crash. Importing the p12 will still fail, but in a safe way.

https://hg.mozilla.org/projects/nss/rev/8872ebb63acdaf69cbe3a134c1aeb2c223e3eaaa
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → 3.38
Comment on attachment 8976067 [details]
Bug 1460673 - don't crash

Tim Taubert [:ttaubert] has approved the revision.

https://phabricator.services.mozilla.com/D1295
Attachment #8976067 - Flags: review+
How far back does this issue go?
Flags: needinfo?(franziskuskiefer)
(In reply to Ryan VanderMeulen [:RyanVM] from comment #9)
> How far back does this issue go?

AFAICT, to whenever the P7 code was originally written for Netscape. It's present in March 2000 for the first checkin [1].


[1] https://hg.mozilla.org/projects/nss/file/cd12bbc0e0a9/security/nss/lib/pkcs7/p7decode.c#l1200
Flags: needinfo?(franziskuskiefer)
If we end up spinning updated NSS releases for the TLS 1.3 issue (bug 1462303) anyway, would it make sense to also include this as a ride-along?
Flags: needinfo?(franziskuskiefer)
It's probably good to get it into ESR60. I don't know that it's important to get into a 60 point release, but it would make sense to have it ride-along anyway.

(Franziskus is on PTO for the next week, so please NI me or :keeler if needed on this one now)
Flags: needinfo?(franziskuskiefer)
Group: crypto-core-security → core-security-release
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.