Skip to content

fix: cap in-memory session storage in /init_memory to prevent DoS (CWE-400)#69

Open
sebastiondev wants to merge 1 commit intoBAI-LAB:mainfrom
sebastiondev:fix/cwe400-app-unbounded-bd53
Open

fix: cap in-memory session storage in /init_memory to prevent DoS (CWE-400)#69
sebastiondev wants to merge 1 commit intoBAI-LAB:mainfrom
sebastiondev:fix/cwe400-app-unbounded-bd53

Conversation

@sebastiondev
Copy link
Copy Markdown

Summary

The /init_memory endpoint in memoryos-playground/memdemo/app.py previously stored a new Memoryos instance in the global memory_systems dict for every request, with no cap on the number of entries. Because the endpoint requires no authentication and the demo app binds to 0.0.0.0:5019, an unauthenticated remote attacker can repeatedly POST to /init_memory with arbitrary user_id values and exhaust server memory — each entry holds a Memoryos instance with an LLM client and conversation buffers.

  • CWE: CWE-400 (Uncontrolled Resource Consumption)
  • File: memoryos-playground/memdemo/app.py
  • Endpoint: POST /init_memory
  • Preconditions: network reachability to port 5019; no auth required.

Data flow

  1. Unauthenticated client sends POST /init_memory with attacker-controlled user_id, api_key, base_url, model.
  2. init_memory() constructs a new Memoryos(...) and stores it in the module-global memory_systems[session_id].
  3. Nothing ever removes entries. Each call grows the dict without bound until the process is OOM-killed.

Fix

Cap the in-memory session store and add automatic eviction:

  • Convert memory_systems to an OrderedDict so we can evict in FIFO order.
  • Add a parallel _session_timestamps OrderedDict to track creation time.
  • On every /init_memory call:
    1. Evict any session older than SESSION_TTL_SECONDS (1 hour).
    2. If the store is at MAX_SESSIONS (100), evict the oldest entry.
    3. If still full after eviction, return HTTP 503 instead of growing the dict.

This bounds in-memory state to a constant, regardless of request volume.

Test results

  • Verified the patch applies cleanly and the file parses (python3 -m py_compile memoryos-playground/memdemo/app.py).
  • Walked through the eviction logic by hand for the boundary cases: empty store, store at capacity with all entries fresh (FIFO eviction kicks in), store at capacity with expired entries (TTL sweep frees space first), and repeated requests at steady state (count stays at MAX_SESSIONS).
  • Existing behavior for legitimate single-user flow is unchanged: the same session_id is generated, stored, and returned to the client; the only addition is a timestamp entry.

Security analysis

The endpoint is reachable without authentication and there is no rate limiting in front of it, so the attack is a single unauthenticated HTTP loop. Before the fix, each request permanently adds a Memoryos instance to a process-global dict; the rate of growth is bounded only by how fast the attacker can issue requests. After the fix, the resident set of session objects is bounded by MAX_SESSIONS (100), and the oldest entries are released on every new request, so the worst case is constant memory rather than unbounded.

Honest caveats for the maintainer:

  • The fix bounds in-memory growth. It does not clean up on-disk state (e.g. chromadb directories) created per user_id, so a disk-exhaustion variant of the same attack remains and is worth tracking separately.
  • FIFO eviction means an attacker who floods /init_memory can push out legitimate sessions, converting a memory-exhaustion DoS into a session-availability DoS. That is a strictly better failure mode than crashing the process, but it is not a complete defense — proper authentication and per-IP rate limiting on this endpoint would be the right longer-term fix.

Adversarial review

Before submitting, we tried to disprove this finding. We checked whether Flask's session cookie, the secret_key, or any upstream proxy in the demo provided implicit authentication or rate limiting on /init_memory — none do; the route is fully open and the dict write happens before any session is established. We also considered whether the demo's debug=True, host='0.0.0.0' posture means it's "not intended for production" and therefore not worth fixing, but the app ships as a runnable playground and several deployment guides point users straight at it, so a bounded-resource default is the right behavior here.

cc @lewiswigmore

…400)

Add MAX_SESSIONS limit (100) and FIFO eviction to the global memory_systems
dict in the playground Flask app. Each /init_memory call previously added an
entry with no upper bound, allowing an attacker to exhaust server memory.

Changes:
- Replace plain dict with OrderedDict for insertion-order eviction
- Add MAX_SESSIONS cap (100) checked before creating a new Memoryos instance
- Add SESSION_TTL_SECONDS (3600) for time-based cleanup of stale sessions
- Evict oldest session when cap is reached, return 503 if still full
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant