*critical* bug

[ctpo6]

It's a really very dangerous bug in the core of library: vmime::object and vmime::net::folderAttributes classes have a flawed copy ctor. The same with operator=(). Because vmime::object is inherited from enable_shared_from_this, it's copy ctor operator=() MUST call inherited ones, but they don't, which leads to garbaged inner weak_ptr with unpredictable consequences. I had got a very nasty memleak in a daemon, not detectable even with valgrind. In my fork of vmime, which I'd made C++11 only, I defined copy ctor and operator=() of these classes as =default.

[vincent-richard]

Hello!

I have addressed some issues with "enable_shared_from_this", and also moved it to the only classes where it is actually required (mainly the ones in the "net" namespace). I have also fixed some ambiguities about how some classes should be instantiated because of this (heap/stack).

Here is the related commit: b1c2d4b

[7i77an]

Vincent,

I'm test this commit and crash my service. This happens when I load my certificates

Before commit, this worked well

vmime::shared_ptrvmime::security::cert::X509Certificate CopyCert(vmime::shared_ptrvmime::security::cert::X509Certificate cert) { stringstream ss; vmime::utility::outputStreamAdapter out(ss); cert->write(out, vmime::security::cert::X509Certificate::FORMAT_PEM); vmime::utility::inputStreamAdapter in(ss); return vmime::security::cert::X509Certificate::import (in); }

** vmime::shared_ptrvmime::security::cert::defaultCertificateVerifier CCertificateHandler::getCertificateVerifier(const std::string& url) { vmime::shared_ptr vrf = vmime::make_shared(url);

boost::mutex::scoped_lock lock (m_mutex);

std::vector<vmime::shared_ptr<vmime::security::cert::X509Certificate> > certificados;   
for (unsigned int i = 0; i < m_certificates.size(); i++) {
    certificados.push_back(CopyCert(m_certificates[i]));
}
vrf->setX509RootCAs(certificados); <= THIS LINE =>
vrf->setX509TrustedCerts(certificados);
return vrf; 

} **

This is the core:

0 0x0000000000440df6 in boost::detail::atomic_exchange_and_add (pw=0x22203a2274726f78, dv=-1)

at /usr/local/include/boost-1_44/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:50

50 /usr/local/include/boost-1_44/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp: No such file or directory. in /usr/local/include/boost-1_44/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp (gdb) bt

0 0x0000000000440df6 in boost::detail::atomic_exchange_and_add (pw=0x22203a2274726f78, dv=-1)

at /usr/local/include/boost-1_44/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:50

1 0x0000000000440f05 in boost::detail::sp_counted_base::release (this=0x22203a2274726f70)

at /usr/local/include/boost-1_44/boost/smart_ptr/detail/sp_counted_base_gcc_x86.hpp:143

2 0x0000000000440f9f in ~shared_count (this=0x7f725c7755e8, __in_chrg=) at /usr/local/include/boost-1_44/boost/smart_ptr/detail/shared_count.hpp:217

3 0x000000000047158c in ~shared_ptr (this=0x7f725c7755e0, __in_chrg=) at /usr/local/include/boost-1_44/boost/smart_ptr/shared_ptr.hpp:169

4 0x0000000000472342 in boost::shared_ptrvmime::security::cert::X509Certificate::operator=(boost::shared_ptrvmime::security::cert::X509Certificate const&) ()

5 0x000000000047518e in boost::shared_ptrvmime::security::cert::X509Certificate* std::copy_move<false, false, std::random_access_iterator_tag>::copy_m<boost::sharedptr< vmime::security::cert::X509Certificate> const, boost::sharedptrvmime::security::cert::X509Certificate>(boost::sharedptrvmime::security::cert::X509Certificate const, boo st::sharedptrvmime::security::cert::X509Certificate const, boost::sharedptrvmime::security::cert::X509Certificate) ()

6 0x00000000004749bb in boost::sharedptrvmime::security::cert::X509Certificate std::__copy_move_a<false, boost::sharedptr const, boost::sharedptrvmime::security::cert::X509Certificate>(boost::sharedptrvmime::security::cert::X509Certificate const, boost::sharedptr const, boost::sharedptrvmime::security::cert::X509Certificate) ()

7 0x0000000000473f67 in gnu_cxx::__normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate, std::vector<boost::shared_ptr, std::allocatorboost::shared_ptr > > std::copy_move_a2<false, gnu_cxx::__normal_iterator<boost::sharedptr const, std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::allocatorboost::shared_ptr > >, gnu_cxx::normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate, std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::alloc atorboost::shared_ptr > > >(__gnu_cxx::normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate const, std::vector boost::shared_ptr<vmime::security::cert::X509Certificate, std::allocatorboost::shared_ptr > >, gnu_cxx::__normal_iterator<boost::s haredptr const, std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::allocator<boost::shared_ptr > > >, gnu_cxx::__normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate, std::vector<boost::shared_ptr<vmime::security::cert::X50 9Certificate>, std::allocatorboost::shared_ptr > >) ()

8 0x0000000000473400 in gnu_cxx::__normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate, std::vector<boost::shared_ptr, std::allocatorboost::shared_ptr > > std::copy<gnu_cxx::normal_iterator<boost::sharedptr const, std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::allocatorboost::shared_ptr > >, __gnu_cxx: :normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate, std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::allocator<boost::share d_ptr > > > >(gnu_cxx::__normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate const, std::vector<boost::shared_pt r, std::allocatorboost::shared_ptr > >, gnu_cxx::normal_iterator<boost::sharedptr const, std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::allocator<boost::shared_ptr > > >, __gnu_cxx::normal_iteratorboost::sharedptr<vmime::security::cert::X509Certificate, std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, st d::allocatorboost::shared_ptr > >) ()

9 0x000000000047260b in std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::allocatorboost::shared_ptr >::op erator=(std::vectorboost::shared_ptr<vmime::security::cert::X509Certificate, std::allocatorboost::shared_ptr > const&) ()

10 0x00000000004c4607 in vmime::security::cert::defaultCertificateVerifier::setX509RootCAs (this=0x13cccc0, caCerts=...)

at /root/integration-build/sources/emailproxy/src/libvmime-master/src/vmime/security/cert/defaultCertificateVerifier.cpp:162

11 0x0000000000470eb0 in CCertificateHandler::getCertificateVerifier (this=0x135fa10, url=...) at ../src/CertificateHandler.cpp:151

12 0x0000000000443343 in CAccount::GetStore (this=0x13cbad0) at ../src/Account.cpp:311

13 0x0000000000449268 in CAccount::CheckNewMail (this=0x13cbad0) at ../src/Account.cpp:729

14 0x000000000047f271 in CEMailController::AsyncCheckNewMail (this=0x1358d90, wkptr=...) at ../src/EMailController.cpp:66

15 0x0000000000489acf in boost::_mfi::mf1<void, CEMailController, boost::weak_ptr >::operator() (this=0x13c9c90, p=0x1358d90, a1=...)

at /usr/local/include/boost-1_44/boost/bind/mem_fn_template.hpp:165

16 0x0000000000489359 in boost::_bi::list2<boost::bi::value<CEMailController>, boost::_bi::valueboost::weak_ptr >::operator()<boost::_mfi::mf1<void, CEMailContro ller, boost::weak_ptr >, boost::_bi::list0> (this=0x13c9ca0, f=..., a=...) at /usr/local/include/boost-1_44/boost/bind/bind.hpp:313

17 0x0000000000487de7 in boost::_bi::bind_t<void, boost::_mfi::mf1<void, CEMailController, boost::weak_ptr >, boost::_bi::list2boost::_bi::value<CEMailController*, boost::_bi::valueboost::weak_ptr > >::operator() (this=0x13c9c90) at /usr/local/include/boost-1_44/boost/bind/bind_template.hpp:20

18 0x0000000000485f69 in boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void, boost::_mfi::mf1<void, CEMailController, boost::weak_ptr >, boos t::_bi::list2boost::_bi::value<CEMailController*, boost::_bi::valueboost::weak_ptr > >, void>::invoke (function_obj_ptr=...)

at /usr/local/include/boost-1_44/boost/function/function_template.hpp:153

19 0x00000000004660f1 in boost::function0::operator() (this=0x7f725c7766b0) at /usr/local/include/boost-1_44/boost/function/function_template.hpp:1013

20 0x00000000004607cb in CAsyncCaller::Execute (this=0x1358e70, function=..., timer=...) at ../src/AsyncCaller.cpp:28

[vincent-richard]

Hello! I'm currently investigating on this issue...

[vincent-richard]

I'm not able to reproduce your issue with the latest VMime code from today. Maybe there is some other code of your own that is involved in the problem?

As far as I understand, the crash occurs when copying a std::vector of shared_ptr <X509Certificate>, that's what I tried to reproduce.

Here is the test program I used (also ran it through gdb and Valgrind):

#include <iostream>
#include <fstream>

#include "vmime/vmime.hpp"
#include "vmime/platforms/posix/posixHandler.hpp"

vmime::shared_ptr <vmime::security::cert::X509Certificate> CopyCert(
    vmime::shared_ptr <vmime::security::cert::X509Certificate> cert
) {

    std::stringstream ss;
    vmime::utility::outputStreamAdapter out(ss);
    cert->write(out, vmime::security::cert::X509Certificate::FORMAT_PEM);
    vmime::utility::inputStreamAdapter in(ss);
    return vmime::security::cert::X509Certificate::import(in);
}

int main() {

    const char *certs[] = {
        // Put paths to any certificates here...
        "/usr/share/ca-certificates/mozilla/QuoVadis_Root_CA_3.crt",
        "/usr/share/ca-certificates/mozilla/NetLock_Notary_=Class_A=_Root.crt",
        "/usr/share/ca-certificates/mozilla/SwissSign_Silver_CA_-_G2.crt",
        "/usr/share/ca-certificates/mozilla/TWCA_Root_Certification_Authority.crt",
    };

    // Test adding certificates
    std::vector <vmime::shared_ptr <vmime::security::cert::X509Certificate> > certificates;

    for (unsigned i = 0 ; i < sizeof(certs) / sizeof(certs[0]) ; ++i) {

        std::ifstream is(certs[i]);
        vmime::utility::inputStreamAdapter vis(is);

        vmime::shared_ptr <vmime::security::cert::X509Certificate> cert =
             vmime::security::cert::X509Certificate::import(vis);

        certificates.push_back(CopyCert(cert));
    }

    vmime::shared_ptr <vmime::security::cert::defaultCertificateVerifier> vrf =
        vmime::make_shared <vmime::security::cert::defaultCertificateVerifier>();

    vrf->setX509RootCAs(certificates);
    vrf->setX509TrustedCerts(certificates);

    // Test copying vector
    std::vector <vmime::shared_ptr <vmime::security::cert::X509Certificate> > certificatesCopy = certificates;
    std::cout << "count = " << certificatesCopy.size() << std::endl;

    // Test using object
    std::vector <vmime::shared_ptr <vmime::security::cert::certificate> > certificates2;

    for (unsigned i = 0 ; i < certificates.size() ; ++i) {
        certificates2.push_back(certificates[i]);
    }

    vmime::shared_ptr <vmime::security::cert::certificateChain> chain =
        vmime::make_shared <vmime::security::cert::certificateChain>(certificates2);

    try {
        vrf->verify(chain, "test.com");
    } catch (vmime::exception &e) {
        // verify() will fail
    }

    return 0;
}

[7i77an]

Mmmm ok Vincent, I will review again.