Use after free triggered on application exit

[canatella]

Please fill in the following fields:

Pre-built SDK from the website or open-source from this repo: pre-built Firebase C++ SDK version: 6.15.0 Firebase plugins in use (Auth, Database, etc.): Auth Additional SDKs you are using (Facebook, AdMob, etc.): none Platform you are using the C++ SDK on (Mac, Windows, or Linux): mac Platform you are targeting (iOS, Android, and/or desktop): all

Please describe the issue here:

When the c++ runtime is cleaning on exit, the cleanup call at https://github.com/firebase/firebase-cpp-sdk/blob/d464bcfd26eec7735d7a8374b0cd325e77fc0280/auth/src/auth.cc#L160 causes a use after free memory access:

=================================================================
==6558==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000004450 at pc 0x000106e57cc1 bp 0x7ffee8f89d30 sp 0x7ffee8f89d28
READ of size 8 at 0x604000004450 thread T0
    #0 0x106e57cc0 in void std::__1::__tree_remove<std::__1::__tree_node_base<void*>*>(std::__1::__tree_node_base<void*>*, std::__1::__tree_node_base<void*>*) __tree:354
    #1 0x1076f2655 in firebase::auth::Auth::DeleteInternal() (flib-ui:x86_64+0x100a7d655)
    #2 0x1076f27a8 in firebase::auth::Auth::~Auth() (flib-ui:x86_64+0x100a7d7a8)
    #3 0x1071e574a in std::__1::default_delete<firebase::auth::Auth>::operator()(firebase::auth::Auth*) const memory:2338
    #4 0x1071e56ce in std::__1::unique_ptr<firebase::auth::Auth, std::__1::default_delete<firebase::auth::Auth> >::reset(firebase::auth::Auth*) memory:2593
    #5 0x1071e5608 in std::__1::unique_ptr<firebase::auth::Auth, std::__1::default_delete<firebase::auth::Auth> >::~unique_ptr() memory:2547
    #6 0x1071e5284 in std::__1::unique_ptr<firebase::auth::Auth, std::__1::default_delete<firebase::auth::Auth> >::~unique_ptr() memory:2547
    #7 0x1071e5190 in my_ns::backend_firebase::~backend_firebase() backend_firebase.hpp:33

This is more or less the way I create and use the auth object:

class backend {
    std::unique_ptr<App> app;
    std::unique_ptr<Auth> auth;
    static std::shared_ptr<backend> instance;

public:
    std::shared_ptr<backend> get_instance() {
        if (instance == nullptr) {
            instance = std::make_shared<backend>();
            instance.app = std::unique_ptr<App>(App::Create(...));
            firebase::InitResult result{};
            auto *auth = auth::Auth::GetAuth(instance.app.get(), &result);
            assert(result == firebase::kInitResultSuccess);
            instance.auth = std::unique_ptr<auth::Auth>(auth);
        }
        return instance;
    }    
}

So I lazy load the auth to avoid loading firebase for part of my application that doesn't use it. On exit the c++ runtime will cleanup globals in the reverse order that they were created. It happens that in my case the runtime first instanciates the backend std::shared_ptr with nullptr as value, then the firebase g_auths global. Then I call get_instance and the shared ptr is now holding a backend instance. Then I exit the application. The c++ runtime then first destructs the g_auths and then destructs my backend instance which in turn destructs the auth instance which tries to remove it self from the already destructed g_auths map.

I'm now working around this by setting the backend instance static global to nullptr on exit but it kind of ruin the point of using smart pointers. Maybe the g_auth map could be part of some global context that I could also lazy load in a smart pointer. Anyway, not a big problem at all but I wanted to let you know of my use case and the problem with the current api. Also maybe you have a better solution for me: if so, I'm all ears ;) Thank you!

Please answer the following, if applicable:

Have you been able to reproduce this issue with just the Firebase C++ quickstarts ? n/a

What's the issue repro rate? (eg 100%, 1/5 etc) 100%

[chkuang-g]

@canatella

Thank you for reporting this.

This seems to be a valid issue when Auth or any Firebase instance is held by a static smart pointer. As you described, if g_auths is destroyed before Auth instance, accessing g_auths and g_auths_mutex would be pretty bad in Auth::DeleteInternal().

Perhaps making g_auth a pointer like what we do in database. https://github.com/firebase/firebase-cpp-sdk/blob/d464bcfd26eec7735d7a8374b0cd325e77fc0280/database/src/common/database.cc#L62

Still feel pretty nervous to access the mutex though.

I'll report to the team about this issue. May require some refactoring. At the meantime, please try to clear your smart pointers before the app shutdown.

[chkuang-g]

After doing a quick scan, I think Auth probably is the only one that is NOT using a pointer for the map (All app, database, firestore, storage are using pointer).

Perhaps this is where we can start.