1543685 - Crash in cert_storage when preference does not exist

Status: RESOLVED FIXED

(Core :: Security: PSM, defect, P1)

Description

[Mathieu Leplatre [:leplatrem]]

While working on Bug 1512451 I faced some crashes in cert-storage.

This one especially when the preference in the is_blocklist_fresh() function does not exist.

 0:02.31 pid:14143 Hit MOZ_CRASH(overflow when adding duration to time) at src/libcore/option.rs:1008
 0:03.25 pid:14143 #01: gkrust_shared::panic_hook (/home/mathieu/Code/Mozilla/firefox/toolkit/library/rust/shared/lib.rs:240)
 0:03.63 pid:14143 #02: core::ops::function::Fn::call (54uhkbr36hlhaga5:?)
 0:03.68 pid:14143 #03: core::sync::atomic::atomic_sub (/rustc/14997d56a550f4aa99fe737593cd2758227afc56/src/libcore/sync/atomic.rs:2079)
 0:03.70 pid:14143 #04: std::panicking::continue_panic_fmt (std.5xg9hvi2-cgu.0:?)
 0:03.72 pid:14143 #05: rust_begin_unwind (/rustc/14997d56a550f4aa99fe737593cd2758227afc56//src/libstd/panicking.rs:325)
 0:03.75 pid:14143 #06: core::panicking::panic_fmt (/rustc/14997d56a550f4aa99fe737593cd2758227afc56//src/libcore/panicking.rs:95)
 0:03.77 pid:14143 #07: core::option::expect_failed (/rustc/14997d56a550f4aa99fe737593cd2758227afc56//src/libcore/option.rs:1008)
 0:03.78 pid:14143 #08: <core::option::Option<T>>::expect (/rustc/14997d56a550f4aa99fe737593cd2758227afc56/src/libcore/option.rs:322)
 0:03.79 pid:14143 #09: cert_storage::SecurityState::is_data_fresh (/home/mathieu/Code/Mozilla/firefox/security/manager/ssl/cert_storage/src/lib.rs:381)
 0:03.80 pid:14143 #10: cert_storage::SecurityState::is_blocklist_fresh (/home/mathieu/Code/Mozilla/firefox/security/manager/ssl/cert_storage/src/lib.rs:395)
 0:03.80 pid:14143 #11: cert_storage::CertStorage::IsBlocklistFresh (/home/mathieu/Code/Mozilla/firefox/security/manager/ssl/cert_storage/src/lib.rs:923)
 0:03.83 pid:14143 #12: mozilla::psm::NSSCertDBTrustDomain::CheckRevocation(mozilla::pkix::EndEntityOrCA, mozilla::pkix::CertID const&, mozilla::pkix::Time, mozilla::pkix::Duration, mozilla::pkix::Input const*, mozilla::pkix::Input const*) (/home/mathieu/Code/Mozilla/firefox/security/certverifier/NSSCertDBTrustDomain.cpp:477)
 0:03.84 pid:14143 #13: mozilla::pkix::PathBuildingStep::Check(mozilla::pkix::Input, mozilla::pkix::Input const*, bool&) (/home/mathieu/Code/Mozilla/firefox/security/nss/lib/mozpkix/lib/pkixbuild.cpp:254)
 0:03.85 pid:14143 #14: mozilla::psm::NSSCertDBTrustDomain::FindIssuer(mozilla::pkix::Input, mozilla::pkix::TrustDomain::IssuerChecker&, mozilla::pkix::Time) (/home/mathieu/Code/Mozilla/firefox/security/certverifier/NSSCertDBTrustDomain.cpp:165)
 0:03.87 pid:14143 #15: mozilla::pkix::BuildForward(mozilla::pkix::TrustDomain&, mozilla::pkix::BackCert const&, mozilla::pkix::Time, mozilla::pkix::KeyUsage, mozilla::pkix::KeyPurposeId, mozilla::pkix::CertPolicyId const&, mozilla::pkix::Input const*, unsigned int, unsigned int&) (/home/mathieu/Code/Mozilla/firefox/security/nss/lib/mozpkix/lib/pkixbuild.cpp:365)
 0:03.88 pid:14143 #16: mozilla::pkix::BuildCertChain(mozilla::pkix::TrustDomain&, mozilla::pkix::Input, mozilla::pkix::Time, mozilla::pkix::EndEntityOrCA, mozilla::pkix::KeyUsage, mozilla::pkix::KeyPurposeId, mozilla::pkix::CertPolicyId const&, mozilla::pkix::Input const*) (/home/mathieu/Code/Mozilla/firefox/security/nss/lib/mozpkix/lib/pkixbuild.cpp:413)
 0:03.88 pid:14143 #17: mozilla::psm::BuildCertChainForOneKeyUsage(mozilla::psm::NSSCertDBTrustDomain&, mozilla::pkix::Input, mozilla::pkix::Time, mozilla::pkix::KeyUsage, mozilla::pkix::KeyUsage, mozilla::pkix::KeyUsage, mozilla::pkix::KeyPurposeId, mozilla::pkix::CertPolicyId const&, mozilla::pkix::Input const*, mozilla::psm::CertVerifier::OCSPStaplingStatus*) (/home/mathieu/Code/Mozilla/firefox/security/certverifier/CertVerifier.cpp:208)
 0:03.89 pid:14143 #18: mozilla::psm::CertVerifier::VerifyCert(CERTCertificateStr*, long, mozilla::pkix::Time, void*, char const*, std::unique_ptr<CERTCertListStr, mozilla::UniqueCERTCertListDeletePolicy>&, unsigned int, SECItemStr const*, SECItemStr const*, mozilla::OriginAttributes const&, SECOidTag*, mozilla::psm::CertVerifier::OCSPStaplingStatus*, mozilla::psm::KeySizeStatus*, mozilla::psm::SHA1ModeResult*, mozilla::psm::PinningTelemetryInfo*, mozilla::psm::CertificateTransparencyInfo*) (/home/mathieu/Code/Mozilla/firefox/security/certverifier/CertVerifier.cpp:705)
 0:03.93 pid:14143 #19: VerifyCertAtTime(nsIX509Cert*, long, unsigned int, nsTSubstring<char> const&, mozilla::pkix::Time, nsIX509CertList**, bool*, int*) (/home/mathieu/Code/Mozilla/firefox/security/manager/ssl/nsNSSCertificateDB.cpp:1147)
 0:03.95 pid:14143 #20: VerifyCertAtTimeTask::CalculateResult() (/home/mathieu/Code/Mozilla/firefox/security/manager/ssl/nsNSSCertificateDB.cpp:1192)
 0:03.99 pid:14143 #21: mozilla::CryptoTask::Run() (/home/mathieu/Code/Mozilla/firefox/security/manager/ssl/CryptoTask.cpp:36)
 0:04.03 pid:14143 #22: nsThread::ProcessNextEvent(bool, bool*) (/home/mathieu/Code/Mozilla/firefox/xpcom/threads/nsThread.cpp:1167)
 0:04.04 pid:14143 #23: NS_ProcessNextEvent(nsIThread*, bool) (/home/mathieu/Code/Mozilla/firefox/xpcom/threads/nsThreadUtils.cpp:486)
 0:04.10 pid:14143 #24: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) (/home/mathieu/Code/Mozilla/firefox/ipc/glue/MessagePump.cpp:303)
 0:04.12 pid:14143 #25: MessageLoop::RunInternal() (/home/mathieu/Code/Mozilla/firefox/ipc/chromium/src/base/message_loop.cc:315)
 0:04.14 pid:14143 #26: MessageLoop::AutoRunState::~AutoRunState() (/home/mathieu/Code/Mozilla/firefox/ipc/chromium/src/base/message_loop.cc:584)
 0:04.16 pid:14143 #27: nsThread::ThreadFunc(void*) (/home/mathieu/Code/Mozilla/firefox/xpcom/threads/nsThread.cpp:456)
 0:04.27 pid:14143 #28: _pt_root (/home/mathieu/Code/Mozilla/firefox/nsprpub/pr/src/pthreads/ptthread.c:204)
 0:04.36 pid:14143 #29: start_thread (/build/glibc-B9XfQf/glibc-2.28/nptl/pthread_create.c:487 (discriminator 6))
 0:08.65 pid:14143 #30: __GI___clone (/build/glibc-B9XfQf/glibc-2.28/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:97)
 0:08.65 pid:14143 #31: ??? (???:???)
 0:08.65 pid:14143 ExceptionHandler::GenerateDump cloned child 14169
 0:08.65 pid:14143 ExceptionHandler::SendContinueSignalToChild sent continue signal to child
 0:08.65 pid:14143 ExceptionHandler::WaitForContinueSignal waiting for continue signal...
 0:08.65 TEST_END: FAIL, expected PASS - xpcshell return code: -11
PROCESS-CRASH | security/manager/ssl/tests/unit/test_cert_storage.js | application crashed [unknown top frame]

Comment 1

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Attachment: bug 1543685 - handle preference values more safely in cert_storage r?mgoodwin

Previously cert_storage could use negative values as unsigned values when
determining if its data was sufficiently fresh, which could cause assertion
failures when doing time math.
This patch changes the behavior to just use 0 if values are either unavailable
or negative, which means we fail closed and say everything is out of date if we
otherwise don't have the information to make the correct decision.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d6ae901c4f90
handle preference values more safely in cert_storage r=mgoodwin

Comment 3

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/d6ae901c4f90