drogonframework / drogonframework/drogon

Potential HashDoS in the server-side session map from client-supplied session IDs

Open
#2,593 0 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.