[Bug] Celix ServiceTracker returns dangling pointers / UAF on concurrent unregister (deprecated API doesn’t retain useCount)

[yxscc]

Summary

serviceTracker_getService (deprecated) returns the service pointer without retaining tracked->useCount or waiting for ongoing unregister. If another thread unregisters the same service, serviceTracker_untrackTracked waits for useCount to drop to 1, then calls bundleContext_ungetService and frees tracked. Any caller still using the old pointer hits freed memory → UAF.

Component

• File: libs/framework/src/service_tracker.c
• API: serviceTracker_getService (deprecated but still callable)
• Scenario: concurrent “get service” vs “unregister service”

Root Cause

1. serviceTracker_getService only locks the list and returns tracked->service; it does not bump useCount or keep the entry alive.
2. serviceTracker_untrackTracked waits for useCount, then ungets and frees the tracked entry; previously returned raw pointers become dangling.
3. The function is marked “TODO deprecated warning -> not locked” but is neither disabled nor concurrency-safe, so multithreaded callers (common in async callbacks) hit UAF.

Key code excerpts:

// returns raw pointer, no useCount retain
void *serviceTracker_getService(service_tracker_t* tracker) {
    //TODO deprecated warning -> not locked
    void *service = NULL;
    celixThreadMutex_lock(&tracker->state.mutex);
    ... // find tracked->service
    celixThreadMutex_unlock(&tracker->state.mutex);
    return service;
}

// unregister path eventually frees tracked
static void serviceTracker_untrackTracked(...) {
    serviceTracker_invokeRemovingService(tracker, tracked);
    ...
    celixThreadMutex_lock(&tracked->mutex);
    while (tracked->useCount > 1) {
        celixThreadCondition_wait(&tracked->useCond, &tracked->mutex);
    }
    celixThreadMutex_unlock(&tracked->mutex);

    bundleContext_ungetService(tracker->context, tracked->reference, &ungetSuccess);
    bundleContext_ungetServiceReference(tracker->context, tracked->reference);
    assert(tracked->useCount == 1);
    ... // destroy mutex/cond & free(tracked)
}

Reproduction (self-contained, suitable for Issue review)

1. Minimal gtest (drop into any test target):

TEST(ServiceTrackerUafRepro, DeprecatedGetServiceUaf) {
    struct dummy { uint64_t magic; };
    celix_properties_t* props = celix_properties_create();
    celix_properties_setBool(props, CELIX_FRAMEWORK_CLEAN_CACHE_DIR_ON_CREATE, true);
    celix_properties_set(props, CELIX_FRAMEWORK_CACHE_DIR, ".cacheServiceTrackerUaf");
    celix_framework_t* fw = celix_frameworkFactory_createFramework(props);
    auto* ctx = celix_framework_getFrameworkContext(fw);

    auto* svc = static_cast<dummy*>(malloc(sizeof(dummy)));
    svc->magic = 0xBEEFBEEF;
    long svcId = celix_bundleContext_registerService(ctx, svc, "dummy_uaf", nullptr);

    int handle = 0; // non-null handle required
    service_tracker_customizer_pt cust = nullptr;
    ASSERT_EQ(CELIX_SUCCESS, serviceTrackerCustomizer_create(&handle, nullptr, nullptr, nullptr, nullptr, &cust));
    service_tracker_t* tracker = nullptr;
    ASSERT_EQ(CELIX_SUCCESS, serviceTracker_create(ctx, "dummy_uaf", cust, &tracker));
    ASSERT_EQ(CELIX_SUCCESS, serviceTracker_open(tracker));

    std::atomic<bool> stop{false};
    std::thread tGet([&] {
        while (!stop.load()) {
            auto* s = static_cast<dummy*>(serviceTracker_getService(tracker));
            if (s) { s->magic = 0xCAFEBABECAFED00DULL; }
        }
    });
    std::thread tUnreg([&] {
        for (int i = 0; i < 2000 && !stop.load(); ++i) {
            celix_bundleContext_unregisterService(ctx, svcId);
            free(svc); // provider frees immediately
            svc = static_cast<dummy*>(malloc(sizeof(dummy)));
            svc->magic = 0xDEADBEEF + i;
            svcId = celix_bundleContext_registerService(ctx, svc, "dummy_uaf", nullptr);
        }
        stop.store(true);
    });
    tUnreg.join();
    stop.store(true);
    tGet.join();

    celix_bundleContext_unregisterService(ctx, svcId);
    free(svc);
    serviceTracker_close(tracker);
    serviceTracker_destroy(tracker);
    serviceTrackerCustomizer_destroy(cust);
    celix_frameworkFactory_destroyFramework(fw);
    // Expected: crash / ASan UAF because getService returns a freed object.
}

2. Add the test file to libs/framework/gtest and list it in CELIX_FRAMEWORK_TEST_SOURCES in libs/framework/gtest/CMakeLists.txt.

3. Build & run (Debug/ASan recommended):

cmake -S . -B build -DCMAKE_BUILD_TYPE=Debug -DENABLE_TESTING=ON -DBUILD_TESTING=ON
cmake --build build --target test_framework -j4
cd build && ./libs/framework/gtest/test_frameworkd --gtest_filter=*ServiceTrackerUaf*

4. Expected result: runtime crash like free(): invalid size (exit 134) or ASan reports use-after-free; this shows serviceTracker_getService returns a dangling pointer when unregister races.

Impact

• Callers may get freed service pointers under concurrency, leading to crashes or wrong behavior.
• Misuse risk is high because the deprecated API is still callable and older code may rely on it.

[PengZheng]

Yes, serviceTracker_getService is buggy, and will be removed in the coming release.

[rlenferink]

@PengZheng would it be an idea to link the applicable tickets to the correct release milestone (I assume 3.0.0) for users to know upfront what will be fixed in the next release?