Skip to content

Commit 7e683da

Browse files
committed
Fix use-after-free: copy cache data while holding lock
The matched_ep pointer referenced data inside route_cache_list that could be invalidated by another thread (via invalidate_route_cache or LRU eviction) after the cache mutex was released. Fix by copying url_pars and chunk_positions vectors while holding the cache lock, so parameter extraction afterwards uses owned data. For cache miss path, the registered_resources_mutex shared lock is still held so direct reference is safe, but copy for consistency.
1 parent 96ac18f commit 7e683da

File tree

1 file changed

+15
-9
lines changed

1 file changed

+15
-9
lines changed

src/webserver.cpp

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -757,16 +757,22 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct details
757757
if (regex_checking) {
758758
details::http_endpoint endpoint(st_url, false, false, false);
759759

760+
// Data needed for parameter extraction after match.
761+
// On cache hit, we copy these while holding the cache lock
762+
// to avoid use-after-free if another thread invalidates cache.
763+
vector<string> matched_url_pars;
764+
vector<int> matched_chunks;
765+
760766
// Check the LRU route cache first
761-
const details::http_endpoint* matched_ep = nullptr;
762767
{
763768
std::lock_guard<std::mutex> cache_lock(route_cache_mutex);
764769
auto cache_it = route_cache_map.find(mr->standardized_url);
765770
if (cache_it != route_cache_map.end()) {
766771
// Cache hit — move to front of LRU list
767772
route_cache_list.splice(route_cache_list.begin(), route_cache_list, cache_it->second);
768773
const route_cache_entry& cached = cache_it->second->second;
769-
matched_ep = &cached.matched_endpoint;
774+
matched_url_pars = cached.matched_endpoint.get_url_pars();
775+
matched_chunks = cached.matched_endpoint.get_chunk_positions();
770776
hrm = cached.resource;
771777
found = true;
772778
}
@@ -792,7 +798,9 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct details
792798
}
793799

794800
if (found) {
795-
matched_ep = &found_endpoint->first;
801+
// Safe to reference: registered_resources_mutex (shared) is still held
802+
matched_url_pars = found_endpoint->first.get_url_pars();
803+
matched_chunks = found_endpoint->first.get_chunk_positions();
796804
hrm = found_endpoint->second;
797805

798806
// Store in LRU cache
@@ -810,13 +818,11 @@ MHD_Result webserver::finalize_answer(MHD_Connection* connection, struct details
810818
}
811819

812820
// Extract URL parameters from matched endpoint
813-
if (found && matched_ep != nullptr) {
814-
const auto& url_pars = matched_ep->get_url_pars();
821+
if (found) {
815822
const auto& url_pieces = endpoint.get_url_pieces();
816-
const auto& chunks = matched_ep->get_chunk_positions();
817-
for (unsigned int i = 0; i < url_pars.size(); i++) {
818-
if (chunks[i] >= 0 && static_cast<size_t>(chunks[i]) < url_pieces.size()) {
819-
mr->dhr->set_arg(url_pars[i], url_pieces[chunks[i]]);
823+
for (unsigned int i = 0; i < matched_url_pars.size(); i++) {
824+
if (matched_chunks[i] >= 0 && static_cast<size_t>(matched_chunks[i]) < url_pieces.size()) {
825+
mr->dhr->set_arg(matched_url_pars[i], url_pieces[matched_chunks[i]]);
820826
}
821827
}
822828
}

0 commit comments

Comments
 (0)