From 876142b1b61a077899aea95db602c182ea1d1c8a Mon Sep 17 00:00:00 2001 From: Jeffrey Walton Date: Tue, 15 Aug 2017 03:05:45 -0400 Subject: [PATCH 1/3] Update with latest sources and reproducible build --- cryptest.nmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cryptest.nmake b/cryptest.nmake index b4fe0f2d..8590e716 100644 --- a/cryptest.nmake +++ b/cryptest.nmake @@ -48,9 +48,9 @@ # If you use 'make sources' from Linux makefile, then add 'winpipes.cpp'. Platform specific # classes, like 'rdrand.cpp', should not be included. Add them under the X86 and X64 rules. -LIB_SRCS = cryptlib.cpp cpu.cpp integer.cpp shacal2.cpp md5.cpp shark.cpp zinflate.cpp gf2n.cpp salsa.cpp xtr.cpp oaep.cpp rc2.cpp default.cpp wait.cpp wake.cpp twofish.cpp iterhash.cpp adler32.cpp algparam.cpp marss.cpp blowfish.cpp ecp.cpp strciphr.cpp aria.cpp camellia.cpp dh2.cpp ida.cpp zlib.cpp elgamal.cpp crc.cpp dessp.cpp tea.cpp eax.cpp network.cpp sha.cpp emsa2.cpp pkcspad.cpp squaretb.cpp idea.cpp authenc.cpp hmac.cpp xtrcrypt.cpp queue.cpp mars.cpp rc5.cpp md2.cpp hrtimer.cpp vmac.cpp eprecomp.cpp hex.cpp dsa.cpp fips140.cpp gzip.cpp seal.cpp blake2.cpp files.cpp base32.cpp sharkbox.cpp safer.cpp randpool.cpp sosemanuk.cpp arc4.cpp osrng.cpp skipjack.cpp seed.cpp sha3.cpp filters.cpp bfinit.cpp rabin.cpp 3way.cpp rw.cpp rdtables.cpp rsa.cpp tftables.cpp gost.cpp socketft.cpp nbtheory.cpp panama.cpp modes.cpp rijndael.cpp casts.cpp algebra.cpp esign.cpp gfpcrypt.cpp dll.cpp ec2n.cpp poly1305.cpp polynomi.cpp blumshub.cpp des.cpp basecode.cpp zdeflate.cpp base64.cpp rc6.cpp gf256.cpp mqueue.cpp misc.cpp pssr.cpp channels.cpp rng.cpp threefish.cpp tiger.cpp cast.cpp square.cpp asn.cpp chacha.cpp whrlpool.cpp md4.cpp dh.cpp ccm.cpp mqv.cpp tigertab.cpp gf2_32.cpp cbcmac.cpp ttmac.cpp luc.cpp trdlocal.cpp pubkey.cpp gcm.cpp ripemd.cpp kalyna.cpp kalynatab.cpp keccak.cpp eccrypto.cpp serpent.cpp cmac.cpp winpipes.cpp +LIB_SRCS = cryptlib.cpp cpu.cpp integer.cpp 3way.cpp adler32.cpp algebra.cpp algparam.cpp arc4.cpp aria.cpp asn.cpp authenc.cpp base32.cpp base64.cpp basecode.cpp bfinit.cpp blake2.cpp blowfish.cpp blumshub.cpp camellia.cpp cast.cpp casts.cpp cbcmac.cpp ccm.cpp chacha.cpp channels.cpp cmac.cpp crc.cpp default.cpp des.cpp dessp.cpp dh.cpp dh2.cpp dll.cpp dsa.cpp eax.cpp ec2n.cpp eccrypto.cpp ecp.cpp elgamal.cpp emsa2.cpp eprecomp.cpp esign.cpp files.cpp filters.cpp fips140.cpp fipstest.cpp gcm.cpp gf256.cpp gf2_32.cpp gf2n.cpp gfpcrypt.cpp gost.cpp gzip.cpp hex.cpp hmac.cpp hrtimer.cpp ida.cpp idea.cpp iterhash.cpp kalyna.cpp kalynatab.cpp keccak.cpp luc.cpp mars.cpp marss.cpp md2.cpp md4.cpp md5.cpp misc.cpp modes.cpp mqueue.cpp mqv.cpp nbtheory.cpp network.cpp oaep.cpp osrng.cpp panama.cpp pkcspad.cpp poly1305.cpp polynomi.cpp pssr.cpp pubkey.cpp queue.cpp rabin.cpp randpool.cpp rc2.cpp rc5.cpp rc6.cpp rdrand.cpp rdtables.cpp rijndael.cpp ripemd.cpp rng.cpp rsa.cpp rw.cpp safer.cpp salsa.cpp seal.cpp seed.cpp serpent.cpp sha.cpp sha3.cpp shacal2.cpp shark.cpp sharkbox.cpp skipjack.cpp socketft.cpp sosemanuk.cpp square.cpp squaretb.cpp strciphr.cpp tea.cpp tftables.cpp threefish.cpp tiger.cpp tigertab.cpp trdlocal.cpp ttmac.cpp twofish.cpp vmac.cpp wait.cpp wake.cpp whrlpool.cpp winpipes.cpp xtr.cpp xtrcrypt.cpp zdeflate.cpp zinflate.cpp zlib.cpp -LIB_OBJS = cryptlib.obj cpu.obj integer.obj shacal2.obj md5.obj shark.obj zinflate.obj gf2n.obj salsa.obj xtr.obj oaep.obj rc2.obj default.obj wait.obj wake.obj twofish.obj iterhash.obj adler32.obj algparam.obj marss.obj blowfish.obj ecp.obj strciphr.obj aria.obj camellia.obj dh2.obj ida.obj zlib.obj elgamal.obj crc.obj dessp.obj tea.obj eax.obj network.obj sha.obj emsa2.obj pkcspad.obj squaretb.obj idea.obj authenc.obj hmac.obj xtrcrypt.obj queue.obj mars.obj rc5.obj md2.obj hrtimer.obj vmac.obj eprecomp.obj hex.obj dsa.obj fips140.obj gzip.obj seal.obj blake2.obj files.obj base32.obj sharkbox.obj safer.obj randpool.obj sosemanuk.obj arc4.obj osrng.obj skipjack.obj seed.obj sha3.obj filters.obj bfinit.obj rabin.obj 3way.obj rw.obj rdtables.obj rsa.obj tftables.obj gost.obj socketft.obj nbtheory.obj panama.obj modes.obj rijndael.obj casts.obj algebra.obj esign.obj gfpcrypt.obj dll.obj ec2n.obj poly1305.obj polynomi.obj blumshub.obj des.obj basecode.obj zdeflate.obj base64.obj rc6.obj gf256.obj mqueue.obj misc.obj pssr.obj channels.obj rng.obj threefish.obj tiger.obj cast.obj square.obj asn.obj chacha.obj whrlpool.obj md4.obj dh.obj ccm.obj mqv.obj tigertab.obj gf2_32.obj cbcmac.obj ttmac.obj luc.obj trdlocal.obj pubkey.obj gcm.obj ripemd.obj kalyna.obj kalynatab.obj keccak.obj eccrypto.obj serpent.obj cmac.obj winpipes.obj +LIB_OBJS = cryptlib.obj cpu.obj integer.obj 3way.obj adler32.obj algebra.obj algparam.obj arc4.obj aria.obj asn.obj authenc.obj base32.obj base64.obj basecode.obj bfinit.obj blake2.obj blowfish.obj blumshub.obj camellia.obj cast.obj casts.obj cbcmac.obj ccm.obj chacha.obj channels.obj cmac.obj crc.obj default.obj des.obj dessp.obj dh.obj dh2.obj dll.obj dsa.obj eax.obj ec2n.obj eccrypto.obj ecp.obj elgamal.obj emsa2.obj eprecomp.obj esign.obj files.obj filters.obj fips140.obj fipstest.obj gcm.obj gf256.obj gf2_32.obj gf2n.obj gfpcrypt.obj gost.obj gzip.obj hex.obj hmac.obj hrtimer.obj ida.obj idea.obj iterhash.obj kalyna.obj kalynatab.obj keccak.obj luc.obj mars.obj marss.obj md2.obj md4.obj md5.obj misc.obj modes.obj mqueue.obj mqv.obj nbtheory.obj network.obj oaep.obj osrng.obj panama.obj pkcspad.obj poly1305.obj polynomi.obj pssr.obj pubkey.obj queue.obj rabin.obj randpool.obj rc2.obj rc5.obj rc6.obj rdrand.obj rdtables.obj rijndael.obj ripemd.obj rng.obj rsa.obj rw.obj safer.obj salsa.obj seal.obj seed.obj serpent.obj sha.obj sha3.obj shacal2.obj shark.obj sharkbox.obj skipjack.obj socketft.obj sosemanuk.obj square.obj squaretb.obj strciphr.obj tea.obj tftables.obj threefish.obj tiger.obj tigertab.obj trdlocal.obj ttmac.obj twofish.obj vmac.obj wait.obj wake.obj whrlpool.obj winpipes.obj xtr.obj xtrcrypt.obj zdeflate.obj zinflate.obj zlib.obj TEST_SRCS = bench1.cpp bench2.cpp test.cpp validat0.cpp validat1.cpp validat2.cpp validat3.cpp datatest.cpp regtest1.cpp regtest2.cpp regtest3.cpp fipsalgt.cpp dlltest.cpp fipstest.cpp From 0110f8397f699998022462d081456965edcc18e0 Mon Sep 17 00:00:00 2001 From: Jeffrey Walton Date: Tue, 15 Aug 2017 03:07:30 -0400 Subject: [PATCH 2/3] Add ELEMS_MAX for SecBlock (Issue 346) Reset the mark on additional class methods --- secblock.h | 70 ++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 15 deletions(-) diff --git a/secblock.h b/secblock.h index 6eb3b5cf..a2a695af 100644 --- a/secblock.h +++ b/secblock.h @@ -42,13 +42,29 @@ public: void construct(pointer p, const T& val) {new (p) T(val);} void destroy(pointer p) {CRYPTOPP_UNUSED(p); p->~T();} + //! \brief Returns the maximum number of elements the allocator can provide + //! \details ELEMS_MAX is the maximum number of elements the + //! Allocator can provide. + //! \note In C++03 and below ELEMS_MAX is a static data member of type + //! size_type. In C++11 and above ELEMS_MAX is an enum + //! inheriting from size_type. In both cases ELEMS_MAX can be + //! used before objects are fully constructed, and it does not suffer the + //! limitations of class methods like max_size. + //! \sa Issue 346/CVE-2016-9939 + //! \since Crypto++ 6.0 +#if defined(CRYPTOPP_CXX11) && !defined(CRYPTOPP_DOXYGEN_PROCESSING) + enum : size_type {ELEMS_MAX = SIZE_MAX/sizeof(T)}; +#else + static const size_type ELEMS_MAX = SIZE_MAX/sizeof(T); +#endif + //! \brief Returns the maximum number of elements the allocator can provide //! \returns the maximum number of elements the allocator can provide //! \details Internally, preprocessor macros are used rather than std::numeric_limits //! because the latter is not a constexpr. Some compilers, like Clang, do not //! optimize it well under all circumstances. Compilers like GCC, ICC and MSVC appear //! to optimize it well in either form. - CRYPTOPP_CONSTEXPR size_type max_size() const {return (SIZE_MAX/sizeof(T));} + CRYPTOPP_CONSTEXPR size_type max_size() const {return ELEMS_MAX;} #if defined(CRYPTOPP_CXX11_VARIADIC_TEMPLATES) || defined(CRYPTOPP_DOXYGEN_PROCESSING) @@ -87,7 +103,7 @@ protected: static void CheckSize(size_t size) { // C++ throws std::bad_alloc (C++03) or std::bad_array_new_length (C++11) here. - if (size > (SIZE_MAX/sizeof(T))) + if (size > ELEMS_MAX) throw InvalidArgument("AllocatorBase: requested size would cause integer overflow"); } }; @@ -486,13 +502,29 @@ public: typedef typename A::const_pointer const_iterator; typedef typename A::size_type size_type; + //! \brief Returns the maximum number of elements the block can hold + //! \details ELEMS_MAX is the maximum number of elements the + //! SecBlock can hold. + //! \note In C++03 and below ELEMS_MAX is a static data member of type + //! size_type. In C++11 and above ELEMS_MAX is an enum + //! inheriting from size_type. In both cases ELEMS_MAX can be + //! used before objects are fully constructed, and it does not suffer the + //! limitations of class methods like max_size. + //! \sa Issue 346/CVE-2016-9939 + //! \since Crypto++ 6.0 +#if defined(CRYPTOPP_CXX11) && !defined(CRYPTOPP_DOXYGEN_PROCESSING) + enum : size_type { ELEMS_MAX = A::ELEMS_MAX }; +#else + static const size_type ELEMS_MAX = A::ELEMS_MAX; +#endif + //! \brief Construct a SecBlock with space for size elements. //! \param size the size of the allocation, in elements //! \throws std::bad_alloc //! \details The elements are not initialized. //! \note size is the count of elements, and not the number of bytes explicit SecBlock(size_type size=0) - : m_mark(SIZE_MAX/sizeof(T)), m_size(size), m_ptr(m_alloc.allocate(size, NULLPTR)) { } + : m_mark(ELEMS_MAX), m_size(size), m_ptr(m_alloc.allocate(size, NULLPTR)) { } //! \brief Copy construct a SecBlock from another SecBlock //! \param t the other SecBlock @@ -512,7 +544,7 @@ public: //! Otherwise, the block is empty and not initialized. //! \note size is the count of elements, and not the number of bytes SecBlock(const T *ptr, size_type len) - : m_mark(SIZE_MAX/sizeof(T)), m_size(len), m_ptr(m_alloc.allocate(len, NULLPTR)) { + : m_mark(ELEMS_MAX), m_size(len), m_ptr(m_alloc.allocate(len, NULLPTR)) { CRYPTOPP_ASSERT((!m_ptr && !m_size) || (m_ptr && m_size)); if (ptr && m_ptr) memcpy_s(m_ptr, m_size*sizeof(T), ptr, len*sizeof(T)); @@ -587,17 +619,21 @@ public: //! preserving the streaming interface. The count controls the number of //! elements zeroized, which can be less than size or 0. //! \details An internal variable, m_mark, is initialized to the maximum number - //! of elements. Deallocation triggers a zeroization, and the number of elements - //! zeroized is STDMIN(m_size, m_mark). After zeroization, the memory is - //! returned to the system. + //! of elements. The maximum number of elements is ELEMS_MAX. Deallocation + //! triggers a zeroization, and the number of elements zeroized is + //! STDMIN(m_size, m_mark). After zeroization, the memory is returned to the + //! system. //! \details The ASN.1 decoder uses SetMark() to set the element count to 0 //! before throwing an exception. In this case, the attacker provides a large //! BER encoded length (say 64MB) but only a small number of content octets //! (say 16). If the allocator zeroized all 64MB, then a transient DoS could //! occur as CPU cycles are spent zeroizing unintialized memory. - //! \details If Assign(), New(), Grow(), CleanNew(), CleanGrow() are called, then the - //! count is reset to its default state, which is the maxmimum number of elements. + //! \details Generally speaking, any operation which changes the size of the SecBlock + //! results in the mark being reset to ELEMS_MAX. In particular, if Assign(), + //! New(), Grow(), CleanNew(), CleanGrow() are called, then the count is reset to + //! ELEMS_MAX. The list is not exhaustive. //! \since Crypto++ 6.0 + //! \sa Issue 346/CVE-2016-9939 void SetMark(size_t count) {m_mark = count;} //! \brief Set contents and size from an array @@ -608,8 +644,9 @@ public: void Assign(const T *ptr, size_type len) { New(len); - if (m_ptr && ptr && len) + if (m_ptr && ptr) {memcpy_s(m_ptr, m_size*sizeof(T), ptr, len*sizeof(T));} + m_mark = ELEMS_MAX; } //! \brief Copy contents from another SecBlock @@ -623,9 +660,10 @@ public: if (this != &t) { New(t.m_size); - if (m_ptr && t.m_ptr && t.m_size) + if (m_ptr && t.m_ptr) {memcpy_s(m_ptr, m_size*sizeof(T), t, t.m_size*sizeof(T));} } + m_mark = ELEMS_MAX; } //! \brief Assign contents from another SecBlock @@ -661,6 +699,7 @@ public: memcpy_s(m_ptr+oldSize, (m_size-oldSize)*sizeof(T), m_ptr, oldSize*sizeof(T)); } } + m_mark = ELEMS_MAX; return *this; } @@ -716,7 +755,7 @@ public: { m_ptr = m_alloc.reallocate(m_ptr, m_size, newSize, false); m_size = newSize; - m_mark = SIZE_MAX/sizeof(T); + m_mark = ELEMS_MAX; } //! \brief Change size without preserving contents @@ -731,6 +770,7 @@ public: { New(newSize); if (m_ptr) {memset_z(m_ptr, 0, m_size*sizeof(T));} + m_mark = ELEMS_MAX; } //! \brief Change size and preserve contents @@ -747,8 +787,8 @@ public: { m_ptr = m_alloc.reallocate(m_ptr, m_size, newSize, true); m_size = newSize; - m_mark = SIZE_MAX/sizeof(T); } + m_mark = ELEMS_MAX; } //! \brief Change size and preserve contents @@ -766,8 +806,8 @@ public: m_ptr = m_alloc.reallocate(m_ptr, m_size, newSize, true); memset_z(m_ptr+m_size, 0, (newSize-m_size)*sizeof(T)); m_size = newSize; - m_mark = SIZE_MAX/sizeof(T); } + m_mark = ELEMS_MAX; } //! \brief Change size and preserve contents @@ -781,7 +821,7 @@ public: { m_ptr = m_alloc.reallocate(m_ptr, m_size, newSize, true); m_size = newSize; - m_mark = SIZE_MAX/sizeof(T); + m_mark = ELEMS_MAX; } //! \brief Swap contents with another SecBlock From 659b47108ac42698722aefcd58ba38ace07acc06 Mon Sep 17 00:00:00 2001 From: Jeffrey Walton Date: Tue, 15 Aug 2017 03:11:03 -0400 Subject: [PATCH 3/3] Fix Address Sanitizer findings on GCC117 GCC117 is a Aarch64/ARM64 server with AMD's ARM chip and GCC 7.10. It looks like GCC is performing some std::string optimizations that generates a finding. We did not witness the finding on other platforms, like other Aarch64 devices and x86_64. We will need to check if taking the address of element-0 is still approved way to get the non-const pointer to the elements --- validat0.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/validat0.cpp b/validat0.cpp index 772e4414..6ee466bd 100644 --- a/validat0.cpp +++ b/validat0.cpp @@ -242,13 +242,13 @@ bool TestCompressors() // Unzip random data. See if we can induce a crash for (unsigned int i = 0; i<128; i++) { - SecByteBlock src; + SecByteBlock src; unsigned int len = GlobalRNG().GenerateWord32(4, 0xfff); - src.resize(len); + src.resize(len); RandomNumberSource(GlobalRNG(), len, true, new ArraySink(src, src.size())); src[0] = (byte)0x1f; // magic header - src[1] = (byte)0x8b; + src[1] = (byte)0x8b; src[2] = 0x00; // extra flags src[3] = src[3] & (2 | 4 | 8 | 16 | 32); // flags @@ -313,13 +313,13 @@ bool TestCompressors() // Inflate random data. See if we can induce a crash for (unsigned int i = 0; i<128; i++) { - SecByteBlock src; + SecByteBlock src; unsigned int len = GlobalRNG().GenerateWord32(4, 0xfff); - src.resize(len); + src.resize(len); RandomNumberSource(GlobalRNG(), len, true, new ArraySink(src, src.size())); src[0] = (byte)0x1f; // magic header - src[1] = (byte)0x8b; + src[1] = (byte)0x8b; src[2] = 0x00; // extra flags src[3] = src[3] & (2 | 4 | 8 | 16 | 32); // flags @@ -392,9 +392,9 @@ bool TestCompressors() // Decompress random data. See if we can induce a crash for (unsigned int i = 0; i<128; i++) { - SecByteBlock src; + SecByteBlock src; unsigned int len = GlobalRNG().GenerateWord32(4, 0xfff); - src.resize(len); + src.resize(len); RandomNumberSource(GlobalRNG(), len, true, new ArraySink(src, src.size())); // CMF byte @@ -536,7 +536,7 @@ bool TestEncryptors() StringSource(src, true, new LegacyEncryptor(pwd.c_str(),new StringSink(dest))); StringSource(dest, true, new LegacyDecryptor(pwd.c_str(),new StringSink(rec))); - + if (src != rec) throw Exception(Exception::OTHER_ERROR, "LegacyEncryptor failed a self test"); }