drogonframework / drogonframework/drogon
Potential HashDoS in the server-side session map from client-supplied session IDs
- Dominant language
- C++
- Stars
- 14.3k
- Forks
- 1.4k
- Avg merge
- 1d 13h
- Merged PRs (30d)
- 14
Description
I found that when sessions are enabled, Drogon reads `JSESSIONID` (or the configured cookie name) from the request and passes any non-empty value directly to `SessionManager::getSession`. A missing entry is created in a `CacheMap` backed by default-hashed `std::unordered_map`. On standard libraries with deterministic `std::hash`, chosen same-bucket cookie values can degrade lookup/insertion toward O(n) per operation.
## Affected code
- `lib/src/HttpAppFrameworkImpl.cc:707–720`: reads the session cookie and generates a server-side ID only if the parsed value is empty.
- `lib/src/SessionManager.cc:92–113`: uses the supplied ID as the cache key and creates a session if it is absent.
- `lib/inc/drogon/CacheMap.h:312–336,450`: performs lookup and insertion under `mtx_` in `std::unordered_map`, which defaults to `std::hash` for this cache.
The request's cookie dictionary uses `SafeStringMap` (`lib/src/HttpRequestImpl.h:739`), but the session cache does not. Here, the cookie **value** becomes a key in a different map.
## Relevant code
```cpp
std::string sessionId = req->getCookie(sessionCookieKey_);
// ... only an empty ID is replaced ...
sessionManagerPtr_->getSession(sessionId, needSetSessionid);
sessionMapPtr_->modify(sessionID, /* ... */);
std::unordered_map map_;
```
## Preconditions and impact
Session support is disabled by default. Once enabled, requests reaching session handling can create entries using arbitrary non-empty parsed cookie values; they need not reference an existing session. This handling precedes pre-routing advice and route filters (`lib/src/HttpServer.cc:436–446`), although an earlier synchronous advice or upstream component can reject requests before this point.
The application shares one session cache across requests and connections (`lib/src/HttpAppFrameworkImpl.cc:633–641`). Distinct IDs can therefore accumulate across many requests; one request need not contain many cookies. The default timeout when enabling sessions is zero, which disables automatic expiry. A positive timeout expires inactive entries and is refreshed on access. This session path has no live-session count cap.
For accepted cookie values chosen to collide under the target standard library and bucket layout, operations can approach O(n) as the collision chain grows. Rehashing must be accounted for when constructing such a workload. Longer operations hold the shared cache mutex and can delay other requests accessing sessions. Persisting arbitrary IDs also allows memory growth independently of hash collisions.
## Suggested fix
- Generate a fresh server-side ID when a cookie does not reference an existing session; do not persist arbitrary unknown IDs.
- Use a collision-resistant keyed hash for the backing map, and bound session count and lifetime. Limit session-ID length as an additional resource bound.
Contributor guide
Research direction
Start by tracing session-cookie handling in lib/src/HttpAppFrameworkImpl.cc:707–720 and session creation in lib/src/SessionManager.cc:92–113, then inspect CacheMap.h:312–336,450 and the session-cache setup at HttpAppFrameworkImpl.cc:633–641. Add regression coverage showing that unknown client-supplied IDs cannot grow the cache without bounds and that the selected mitigation preserves valid session behavior; verify request handling and resource limits under the relevant session configuration.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100