yxscc opened a new issue, #811:
URL: https://github.com/apache/celix/issues/811
### 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:
```20:78:libs/framework/src/service_tracker.c
// 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;
}
```
```502:575:libs/framework/src/service_tracker.c
// 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):
```cpp
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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
