fix(security): address 4 High findings from 2026-05-19 audit#125
Closed
Taure wants to merge 4 commits into
Closed
Conversation
H1: WebSocket chat.join and chat.send now run asobi_chat_acl:authorized/2 before joining the pg group or forwarding the message. Without this check any authenticated player could join dm:<alice>:<bob> via the WS path (the HTTP /api/v1/chat history endpoint already enforced membership) and read every subsequent DM between the two parties. H2: New asobi_body_cap_plugin caps HTTP request body size to 1 MiB and rejects chunked POSTs without content-length. Runs before nova_request_plugin so oversized bodies are never buffered into BEAM heap. Per-route caps (256 KB on saves, etc) remain on top of this floor. H3: list_worlds_cached/0,1 backs WS world.list and the HTTP index route with a 500 ms TTL cache owned by asobi_world_lobby_server. A 60 msg/sec WS flood of world.list against 1000 worlds previously fanned out to 60k synchronous get_info calls per second; the cache absorbs the fan-out without breaking find_or_create_unsafe (which stays uncached). H4: Bump nova to 0.14.3 and add explicit override on the cowboy package so cowlib resolves to 2.16.1, clearing the rebar3 audit HIGH advisory against 2.16.0. One LOW cowlib advisory remains pending an upstream cowlib release. Audit doc: docs/security_audit_2026_05_19.md.
GHSA-g2wm-735q-3f56 is a LOW-severity advisory against cow_cookie:cookie/1 (cowlib) with no upstream patch on any released cowlib version, and no patch on ninenines/cowlib master (only a doc update). asobi never calls cow_cookie:cookie/1 — only setcookie via cowboy_req — so the advisory does not apply to this codebase. Track in docs/security_audit_2026_05_19.md. Requires Taure/erlang-ci#62 (audit-ignores input). Temporarily pinned to the feature branch SHA; Dependabot will repin to main after merge.
Contributor
Author
|
Superseded by #167. This branch was 8 weeks stale and its H4 (nova/cowlib) changes would now downgrade cowlib — main already carries |
Taure
added a commit
that referenced
this pull request
Jul 15, 2026
…125) (#167) * fix(security): address H1-H3 High findings from 2026-05-19 audit H1 - route WS chat.join/chat.send through asobi_chat_acl:authorized/2, the shared DM/world/group predicate the HTTP history endpoint already uses. Without it any authenticated player could join dm:<a>:<b> and eavesdrop. ACL logic extracted from asobi_chat_controller into asobi_chat_acl and consumed by both paths. H2 - asobi_body_cap_plugin rejects HTTP bodies > 1 MiB and chunked requests without content-length before nova_request_plugin buffers them into the heap. Wired as the first pre_request plugin. H3 - asobi_world_lobby:list_worlds_cached/0,1 caches world.list for 500ms via an ETS table owned by asobi_world_lobby_server, stopping a WS flood from fanning out synchronous get_info calls. find_or_create stays uncached. H4 (nova/cowlib bump) already landed on main independently - main is on cowlib 2.17.1, newer than this audit's 2.16.1 target, so the H4 rebar changes are intentionally dropped. * fix(security): harden H2/H3 per guardian + security review Review of the H1-H3 re-land (asobi-architecture-guardian, beam-security-reviewer, erlang-code-reviewer) surfaced two defects in the fixes themselves plus test gaps: H3 (High) - the cache was keyed on the raw, attacker-controlled filter map. `mode` is unbounded, so a client cycling distinct modes forced a miss on every request (defeating the cache) and grew the ETS table without bound - a new memory-DoS inside a DoS mitigation. Now keyed on the has_capacity boolean only (<=2 rows); mode is applied in-memory on the cached list. Also bound `mode` to 64 bytes in the HTTP controller to match the WS path. H3 (Low, defence-in-depth) - cache table was `public`; any in-node process could poison the shared world list. Now `protected` and written only by asobi_world_lobby_server via a new cache_worlds/3 cast; callers still read directly. H2 (Medium) - reject/4 returned {break,...}, which nova maps to a 2-tuple {ok,Req} that cowboy's stream handler has no clause for -> case_clause crash of the request process on every 411/413 (log/CPU amplification). Now returns {stop,...}, which cowboy handles cleanly. Tests: cache tests rewritten for the bounded-key scheme incl. a distinct-modes-do-not-grow-table assertion; body-cap tests assert {stop}; new asobi_chat_ws_SUITE covers the H1 WS wiring end-to-end (third-party dm join/send rejected, member join allowed). Fixed a Unicode 'x' in a doc comment (ASCII only).
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
1 participant
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.
Closes the four High findings from the 2026-05-19 security audit.
Summary
chat.join/chat.sendnow route throughasobi_chat_acl:authorized/2, the same predicate the HTTP history endpoint already uses. Without this, any authenticated player could silently joindm:<alice>:<bob>and eavesdrop on every DM between them.asobi_body_cap_pluginrejects HTTP bodies > 1 MiB and chunked requests withoutcontent-length, beforenova_request_pluginbuffers them into the BEAM heap. Per-route caps (e.g. 256 KB on saves) still apply on top.asobi_world_lobby:list_worlds_cached/0,1cachesworld.listresults for 500 ms via an ETS table owned byasobi_world_lobby_server. Stops a 60 msg/sec WS flood from fanning out 60k synchronousget_infocalls per second against running worlds.find_or_create_unsafestays uncached.novato0.14.3and add explicitcowboyoverride socowlibresolves to2.16.1.rebar3 auditnow reports 1 vuln (LOW) down from 3 (with a HIGH on cowlib 2.16.0). The remaining LOW is pending an upstream cowlib release.Test plan
rebar3 eunit— 274 tests, 0 failures (includes 11 new tests acrossasobi_chat_acl_tests,asobi_body_cap_plugin_tests,asobi_world_lobby_cache_tests)rebar3 fmt --checkcleanrebar3 xrefcleanrebar3 dialyzercleanelp eqwalize-all— 0 errors (down from 2 pre-existing inasobi_world_lobby_ws_SUITE, also fixed)elp lint— no new warningsrebar3 audit— 3 vulns → 1 (LOW) remainingWhat is NOT in this PR
The audit's 10 Medium and 6 Low findings are deferred to a follow-up. Mediums worth flagging:
put_storagesize cap)