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
Open
fix: cap in-memory session storage in /init_memory to prevent DoS (CWE-400)#69sebastiondev wants to merge 1 commit intoBAI-LAB:mainfrom
sebastiondev wants to merge 1 commit intoBAI-LAB:mainfrom
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
/init_memoryendpoint inmemoryos-playground/memdemo/app.pypreviously stored a newMemoryosinstance in the globalmemory_systemsdict for every request, with no cap on the number of entries. Because the endpoint requires no authentication and the demo app binds to0.0.0.0:5019, an unauthenticated remote attacker can repeatedly POST to/init_memorywith arbitraryuser_idvalues and exhaust server memory — each entry holds aMemoryosinstance with an LLM client and conversation buffers.memoryos-playground/memdemo/app.pyPOST /init_memoryData flow
POST /init_memorywith attacker-controlleduser_id,api_key,base_url,model.init_memory()constructs a newMemoryos(...)and stores it in the module-globalmemory_systems[session_id].Fix
Cap the in-memory session store and add automatic eviction:
memory_systemsto anOrderedDictso we can evict in FIFO order._session_timestampsOrderedDictto track creation time./init_memorycall:SESSION_TTL_SECONDS(1 hour).MAX_SESSIONS(100), evict the oldest entry.This bounds in-memory state to a constant, regardless of request volume.
Test results
python3 -m py_compile memoryos-playground/memdemo/app.py).MAX_SESSIONS).session_idis 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
Memoryosinstance 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 byMAX_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:
user_id, so a disk-exhaustion variant of the same attack remains and is worth tracking separately./init_memorycan 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'sdebug=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