From 6718f6795214099204e5f9f45549e6bdeb0c1923 Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Wed, 15 Jul 2026 05:31:36 +0200 Subject: [PATCH 1/2] 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:: 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. --- config/dev_sys.config.src | 6 ++ config/prod_sys.config.src | 7 ++ src/controllers/asobi_chat_controller.erl | 71 +------------------ src/controllers/asobi_world_controller.erl | 6 +- src/plugins/asobi_body_cap_plugin.erl | 77 +++++++++++++++++++++ src/social/asobi_chat_acl.erl | 80 ++++++++++++++++++++++ src/world/asobi_world_lobby.erl | 47 +++++++++++++ src/world/asobi_world_lobby_server.erl | 13 ++++ src/ws/asobi_ws_handler.erl | 56 ++++++++++----- test/asobi_body_cap_plugin_tests.erl | 79 +++++++++++++++++++++ test/asobi_chat_acl_tests.erl | 19 +++++ test/asobi_world_lobby_cache_tests.erl | 60 ++++++++++++++++ test/asobi_world_lobby_ws_SUITE.erl | 9 ++- 13 files changed, 439 insertions(+), 91 deletions(-) create mode 100644 src/plugins/asobi_body_cap_plugin.erl create mode 100644 src/social/asobi_chat_acl.erl create mode 100644 test/asobi_body_cap_plugin_tests.erl create mode 100644 test/asobi_chat_acl_tests.erl create mode 100644 test/asobi_world_lobby_cache_tests.erl diff --git a/config/dev_sys.config.src b/config/dev_sys.config.src index 625824f..5c6a010 100644 --- a/config/dev_sys.config.src +++ b/config/dev_sys.config.src @@ -26,6 +26,12 @@ {bootstrap_application, asobi}, {json_lib, json}, {plugins, [ + %% H2 (2026-05-19): cap HTTP body size before nova_request_plugin + %% buffers it into the heap. Default 1 MiB; dev allows the same. + {pre_request, asobi_body_cap_plugin, #{ + max_body => 1048576, + require_content_length => true + }}, {pre_request, nova_request_plugin, #{ decode_json_body => true, parse_qs => true diff --git a/config/prod_sys.config.src b/config/prod_sys.config.src index 14eaa78..078b86f 100644 --- a/config/prod_sys.config.src +++ b/config/prod_sys.config.src @@ -23,6 +23,13 @@ {bootstrap_application, asobi}, {json_lib, json}, {plugins, [ + %% H2 (2026-05-19): cap HTTP body size before nova_request_plugin + %% buffers it into the heap. Default 1 MiB; per-route checks + %% (save/storage) still apply on top of this floor. + {pre_request, asobi_body_cap_plugin, #{ + max_body => 1048576, + require_content_length => true + }}, {pre_request, nova_request_plugin, #{ decode_json_body => true, parse_qs => true diff --git a/src/controllers/asobi_chat_controller.erl b/src/controllers/asobi_chat_controller.erl index 9f4f277..ec57869 100644 --- a/src/controllers/asobi_chat_controller.erl +++ b/src/controllers/asobi_chat_controller.erl @@ -15,7 +15,7 @@ history( ) when is_binary(ChannelId), is_binary(Qs), is_binary(PlayerId) -> - case authorized(ChannelId, PlayerId) of + case asobi_chat_acl:authorized(ChannelId, PlayerId) of true -> Params = cow_qs:parse_qs(Qs), Limit = asobi_qs:integer( @@ -35,72 +35,3 @@ history( end; history(_Req) -> {json, 400, #{}, #{error => ~"invalid_request"}}. - -%% Channel ID schemes (see asobi_dm:channel_id/2 and asobi_world_chat:channel_id/3): -%% dm:: — A and B are the only allowed readers -%% world: — must be currently joined to the world -%% zone::, — must be currently joined to the world -%% prox::, — must be currently joined to the world -%% — treated as a group_id; must be a group member --spec authorized(binary(), binary()) -> boolean(). -authorized(ChannelId, PlayerId) -> - case classify(ChannelId) of - {dm, A, B} -> - PlayerId =:= A orelse PlayerId =:= B; - {world, WorldId} -> - player_in_world(PlayerId, WorldId); - {group, GroupId} -> - is_group_member(PlayerId, GroupId) - end. - --spec classify(binary()) -> {dm, binary(), binary()} | {world, binary()} | {group, binary()}. -classify(<<"dm:", Rest/binary>>) -> - case binary:split(Rest, ~":", [global]) of - [A, B] when byte_size(A) > 0, byte_size(B) > 0 -> {dm, A, B}; - _ -> {group, <<"dm:", Rest/binary>>} - end; -classify(<<"world:", WorldId/binary>>) when byte_size(WorldId) > 0 -> - {world, WorldId}; -classify(<<"zone:", Rest/binary>>) -> - {world, take_until_colon(Rest)}; -classify(<<"prox:", Rest/binary>>) -> - {world, take_until_colon(Rest)}; -classify(ChannelId) -> - {group, ChannelId}. - --spec take_until_colon(binary()) -> binary(). -take_until_colon(Bin) -> - case binary:split(Bin, ~":") of - [Head, _] -> Head; - [Head] -> Head - end. - --spec player_in_world(binary(), binary()) -> boolean(). -player_in_world(PlayerId, WorldId) -> - case asobi_world_server:whereis(WorldId) of - {ok, Pid} -> - try asobi_world_server:get_info(Pid) of - #{players := Players} when is_list(Players) -> - lists:member(PlayerId, Players); - _ -> - false - catch - _:_ -> false - end; - error -> - false - end. - --spec is_group_member(binary(), binary()) -> boolean(). -is_group_member(PlayerId, GroupId) -> - Q = kura_query:where( - kura_query:where( - kura_query:from(asobi_group_member), - {group_id, GroupId} - ), - {player_id, PlayerId} - ), - case asobi_repo:all(Q) of - {ok, [_ | _]} -> true; - _ -> false - end. diff --git a/src/controllers/asobi_world_controller.erl b/src/controllers/asobi_world_controller.erl index 53ccb2a..6925ff9 100644 --- a/src/controllers/asobi_world_controller.erl +++ b/src/controllers/asobi_world_controller.erl @@ -5,10 +5,12 @@ -spec index(map()) -> {json, map()}. index(#{parsed_qs := QS}) -> Filters = build_filters(QS), - Worlds = asobi_world_lobby:list_worlds(Filters), + %% H3 (2026-05-19): use cached enumeration; freshness is 500ms which is + %% well below the granularity a polling client perceives. + Worlds = asobi_world_lobby:list_worlds_cached(Filters), {json, #{worlds => Worlds}}; index(_Req) -> - Worlds = asobi_world_lobby:list_worlds(), + Worlds = asobi_world_lobby:list_worlds_cached(), {json, #{worlds => Worlds}}. -spec show(map()) -> {json, map()} | {status, 404}. diff --git a/src/plugins/asobi_body_cap_plugin.erl b/src/plugins/asobi_body_cap_plugin.erl new file mode 100644 index 0000000..d753f10 --- /dev/null +++ b/src/plugins/asobi_body_cap_plugin.erl @@ -0,0 +1,77 @@ +-module(asobi_body_cap_plugin). +-behaviour(nova_plugin). +-moduledoc """ +Pre-request plugin that caps HTTP request body size. + +Runs before `nova_request_plugin` so we can short-circuit oversized requests +with 413 before any body bytes are buffered into BEAM heap. + +H2 (2026-05-19): without this cap, an authenticated client could POST a +multi-GB JSON body to any `/api/v1/**` endpoint and OOM the node before the +controller's per-route check (e.g. `MAX_SAVE_DATA_BYTES`) ever ran. + +Options: + max_body => non_neg_integer() %% bytes, default 1 MiB + require_content_length => boolean()%% reject chunked w/o content-length, default true +""". + +-export([pre_request/4, post_request/4, plugin_info/0]). + +-define(DEFAULT_MAX_BODY, 1048576). + +-spec pre_request(cowboy_req:req(), map(), map(), term()) -> + {ok, cowboy_req:req(), term()} | {break, cowboy_req:req(), term()}. +pre_request(Req, _Env, Options, State) -> + Max = maps:get(max_body, Options, ?DEFAULT_MAX_BODY), + RequireCL = maps:get(require_content_length, Options, true), + case needs_check(Req) of + false -> + {ok, Req, State}; + true -> + check_size(Req, Max, RequireCL, State) + end. + +-spec post_request(cowboy_req:req(), map(), map(), term()) -> + {ok, cowboy_req:req(), term()}. +post_request(Req, _Env, _Options, State) -> + {ok, Req, State}. + +-spec plugin_info() -> map(). +plugin_info() -> + #{ + title => ~"Body Size Cap", + version => ~"1.0.0", + url => ~"https://github.com/widgrensit/asobi", + authors => [~"widgrensit"], + description => ~"Rejects oversized HTTP request bodies before they are buffered" + }. + +-spec needs_check(cowboy_req:req()) -> boolean(). +needs_check(Req) -> + cowboy_req:has_body(Req). + +-spec check_size(cowboy_req:req(), non_neg_integer(), boolean(), term()) -> + {ok, cowboy_req:req(), term()} | {break, cowboy_req:req(), term()}. +check_size(Req, Max, RequireCL, State) -> + case cowboy_req:body_length(Req) of + undefined when RequireCL -> + reject(411, ~"length_required", Req, State); + undefined -> + {ok, Req, State}; + N when is_integer(N), N > Max -> + reject(413, ~"payload_too_large", Req, State); + _ -> + {ok, Req, State} + end. + +-spec reject(integer(), binary(), cowboy_req:req(), term()) -> + {break, cowboy_req:req(), term()}. +reject(Status, Reason, Req, State) -> + Body = json:encode(#{~"error" => Reason}), + Req1 = cowboy_req:reply( + Status, + #{~"content-type" => ~"application/json"}, + Body, + Req + ), + {break, Req1, State}. diff --git a/src/social/asobi_chat_acl.erl b/src/social/asobi_chat_acl.erl new file mode 100644 index 0000000..50026e9 --- /dev/null +++ b/src/social/asobi_chat_acl.erl @@ -0,0 +1,80 @@ +-module(asobi_chat_acl). +-moduledoc """ +Authorisation policy for chat channels. + +Channel ID schemes: + dm:: - A and B are the only allowed readers + world: - must currently be joined to the world + zone::, - must currently be joined to the world + prox::, - must currently be joined to the world + - treated as a group_id; must be a group member + +Shared by `asobi_chat_controller` (HTTP history) and `asobi_ws_handler` +(WebSocket `chat.join` / `chat.send`). Keeping a single source of truth +prevents the WS path from drifting and silently allowing DM eavesdropping. +""". + +-export([authorized/2]). + +-spec authorized(binary(), binary()) -> boolean(). +authorized(ChannelId, PlayerId) when is_binary(ChannelId), is_binary(PlayerId) -> + case classify(ChannelId) of + {dm, A, B} -> + PlayerId =:= A orelse PlayerId =:= B; + {world, WorldId} -> + player_in_world(PlayerId, WorldId); + {group, GroupId} -> + is_group_member(PlayerId, GroupId) + end. + +-spec classify(binary()) -> {dm, binary(), binary()} | {world, binary()} | {group, binary()}. +classify(<<"dm:", Rest/binary>>) -> + case binary:split(Rest, ~":", [global]) of + [A, B] when byte_size(A) > 0, byte_size(B) > 0 -> {dm, A, B}; + _ -> {group, <<"dm:", Rest/binary>>} + end; +classify(<<"world:", WorldId/binary>>) when byte_size(WorldId) > 0 -> + {world, WorldId}; +classify(<<"zone:", Rest/binary>>) -> + {world, take_until_colon(Rest)}; +classify(<<"prox:", Rest/binary>>) -> + {world, take_until_colon(Rest)}; +classify(ChannelId) -> + {group, ChannelId}. + +-spec take_until_colon(binary()) -> binary(). +take_until_colon(Bin) -> + case binary:split(Bin, ~":") of + [Head, _] -> Head; + [Head] -> Head + end. + +-spec player_in_world(binary(), binary()) -> boolean(). +player_in_world(PlayerId, WorldId) -> + case asobi_world_server:whereis(WorldId) of + {ok, Pid} -> + try asobi_world_server:get_info(Pid) of + #{players := Players} when is_list(Players) -> + lists:member(PlayerId, Players); + _ -> + false + catch + _:_ -> false + end; + error -> + false + end. + +-spec is_group_member(binary(), binary()) -> boolean(). +is_group_member(PlayerId, GroupId) -> + Q = kura_query:where( + kura_query:where( + kura_query:from(asobi_group_member), + {group_id, GroupId} + ), + {player_id, PlayerId} + ), + case asobi_repo:all(Q) of + {ok, [_ | _]} -> true; + _ -> false + end. diff --git a/src/world/asobi_world_lobby.erl b/src/world/asobi_world_lobby.erl index 7d87398..c581059 100644 --- a/src/world/asobi_world_lobby.erl +++ b/src/world/asobi_world_lobby.erl @@ -3,12 +3,15 @@ -export([ list_worlds/0, list_worlds/1, find_or_create/1, find_or_create/2, create_world/1, create_world/2 ]). +-export([list_worlds_cached/0, list_worlds_cached/1]). -export([find_or_create_unsafe/1, find_or_create_unsafe/2]). -export([player_owned_world_count/1, world_capacity_state/1]). -define(PG_SCOPE, nova_scope). -define(DEFAULT_MAX_WORLDS_PER_PLAYER, 5). -define(DEFAULT_MAX_WORLDS, 1000). +-define(LIST_CACHE_TAB, asobi_world_lobby_cache). +-define(LIST_CACHE_TTL_MS, 500). -doc "List all running worlds.". -spec list_worlds() -> [map()]. @@ -40,6 +43,50 @@ list_worlds(Filters) -> ), Worlds. +-doc """ +H3 (2026-05-19): cached variant of `list_worlds/1` for request paths that +do not need a fresh enumeration on every call. Each `list_worlds/1` call +issues one synchronous `gen_server:call` per running world (`get_info/1`); +WS `world.list` at 60 msg/sec × 1000 worlds = 60k calls/sec/attacker. The +cache (500 ms TTL, populated lazily, backed by the ETS table owned by +`asobi_world_lobby_server`) absorbs that fan-out without changing the +serialization story for `find_or_create_unsafe` which stays uncached. +""". +-spec list_worlds_cached() -> [map()]. +list_worlds_cached() -> + list_worlds_cached(#{}). + +-spec list_worlds_cached(map()) -> [map()]. +list_worlds_cached(Filters) -> + Key = Filters, + Now = erlang:monotonic_time(millisecond), + case cache_lookup(Key, Now) of + {hit, Worlds} -> + Worlds; + miss -> + Worlds = list_worlds(Filters), + cache_put(Key, Worlds, Now), + Worlds + end. + +-spec cache_lookup(map(), integer()) -> {hit, [map()]} | miss. +cache_lookup(Key, Now) -> + try ets:lookup(?LIST_CACHE_TAB, Key) of + [{_, Worlds, ExpiresAt}] when ExpiresAt > Now -> {hit, Worlds}; + _ -> miss + catch + error:badarg -> miss + end. + +-spec cache_put(map(), [map()], integer()) -> ok. +cache_put(Key, Worlds, Now) -> + try + ets:insert(?LIST_CACHE_TAB, {Key, Worlds, Now + ?LIST_CACHE_TTL_MS}), + ok + catch + error:badarg -> ok + end. + -doc """ Find a running world with capacity for the given mode, or create one. diff --git a/src/world/asobi_world_lobby_server.erl b/src/world/asobi_world_lobby_server.erl index 36865ea..b7e37a3 100644 --- a/src/world/asobi_world_lobby_server.erl +++ b/src/world/asobi_world_lobby_server.erl @@ -24,6 +24,7 @@ typical deployment supports, the queue stays empty. -export([init/1, handle_call/3, handle_cast/2]). -define(CALL_TIMEOUT, 30000). +-define(LIST_CACHE_TAB, asobi_world_lobby_cache). -spec start_link() -> gen_server:start_ret(). start_link() -> @@ -53,6 +54,18 @@ find_or_create(Mode, PlayerId) -> -spec init([]) -> {ok, #{}}. init([]) -> + %% H3 (2026-05-19): ETS-backed TTL cache for asobi_world_lobby:list_worlds/1. + %% Public read so callers do not have to round-trip this server; only the + %% server creates it so it survives crashes of any single caller. + case ets:info(?LIST_CACHE_TAB, name) of + undefined -> + ?LIST_CACHE_TAB = ets:new(?LIST_CACHE_TAB, [ + set, public, named_table, {read_concurrency, true}, {write_concurrency, true} + ]), + ok; + _ -> + ok + end, {ok, #{}}. -spec handle_call(term(), gen_server:from(), map()) -> diff --git a/src/ws/asobi_ws_handler.erl b/src/ws/asobi_ws_handler.erl index 8b1799c..0fb8ca5 100644 --- a/src/ws/asobi_ws_handler.erl +++ b/src/ws/asobi_ws_handler.erl @@ -217,17 +217,27 @@ handle_message( {ok, State#{session => undefined}} end; handle_message( - #{~"type" := ~"chat.send", ~"payload" := Payload}, #{player_id := PlayerId} = State + #{~"type" := ~"chat.send", ~"payload" := Payload} = Msg, #{player_id := PlayerId} = State ) when is_binary(PlayerId) -> + Cid = maps:get(~"cid", Msg, undefined), #{~"channel_id" := ChannelId, ~"content" := Content} = Payload, case is_binary(Content) andalso byte_size(Content) =< 2000 of - true -> - asobi_chat_channel:send_message(ChannelId, PlayerId, Content), - {ok, State}; false -> - {ok, State} + {ok, State}; + true -> + case + validate_channel_id(ChannelId) andalso + asobi_chat_acl:authorized(ChannelId, PlayerId) + of + true -> + asobi_chat_channel:send_message(ChannelId, PlayerId, Content), + {ok, State}; + false -> + Reply = encode_reply(Cid, ~"error", #{reason => ~"not_authorized"}), + {reply, {text, Reply}, State} + end end; handle_message( #{~"type" := ~"dm.send", ~"payload" := Payload} = Msg, #{player_id := PlayerId} = State @@ -245,26 +255,37 @@ handle_message( {reply, {text, Reply}, State} end; handle_message( - #{~"type" := ~"chat.join", ~"payload" := #{~"channel_id" := ChannelId}} = Msg, State -) when is_binary(ChannelId) -> + #{~"type" := ~"chat.join", ~"payload" := #{~"channel_id" := ChannelId}} = Msg, + #{player_id := PlayerId} = State +) when is_binary(ChannelId), is_binary(PlayerId) -> Cid = maps:get(~"cid", Msg, undefined), %% F-16: bound channel id length, namespace it, and cap how many channels %% one connection may join. Prevents one socket from spawning unbounded %% chat channel processes on the host. + %% H1 (2026-05-19): every join must pass asobi_chat_acl:authorized/2 so a + %% third party cannot silently join `dm::` and eavesdrop. case validate_channel_id(ChannelId) of false -> Reply = encode_reply(Cid, ~"error", #{reason => ~"invalid_channel_id"}), {reply, {text, Reply}, State}; true -> - Joined = maps:get(joined_channels, State, #{}), - case map_size(Joined) >= ?MAX_JOINED_CHANNELS_PER_CONN of - true -> - Reply = encode_reply(Cid, ~"error", #{reason => ~"too_many_channels"}), - {reply, {text, Reply}, State}; + case asobi_chat_acl:authorized(ChannelId, PlayerId) of false -> - asobi_chat_channel:join(ChannelId, self()), - Reply = encode_reply(Cid, ~"chat.joined", #{channel_id => ChannelId}), - {reply, {text, Reply}, State#{joined_channels => Joined#{ChannelId => true}}} + Reply = encode_reply(Cid, ~"error", #{reason => ~"not_authorized"}), + {reply, {text, Reply}, State}; + true -> + Joined = maps:get(joined_channels, State, #{}), + case map_size(Joined) >= ?MAX_JOINED_CHANNELS_PER_CONN of + true -> + Reply = encode_reply(Cid, ~"error", #{reason => ~"too_many_channels"}), + {reply, {text, Reply}, State}; + false -> + asobi_chat_channel:join(ChannelId, self()), + Reply = encode_reply(Cid, ~"chat.joined", #{channel_id => ChannelId}), + {reply, {text, Reply}, State#{ + joined_channels => Joined#{ChannelId => true} + }} + end end end; handle_message( @@ -411,7 +432,10 @@ handle_message( %% bad types. case build_world_filters(Payload) of {ok, Filters} -> - Worlds = asobi_world_lobby:list_worlds(Filters), + %% H3 (2026-05-19): cached variant absorbs the per-world + %% get_info fan-out so a flood of world.list messages cannot + %% stall every world's mailbox. + Worlds = asobi_world_lobby:list_worlds_cached(Filters), Reply = encode_reply(Cid, ~"world.list", #{worlds => Worlds}), {reply, {text, Reply}, State}; {error, Reason} -> diff --git a/test/asobi_body_cap_plugin_tests.erl b/test/asobi_body_cap_plugin_tests.erl new file mode 100644 index 0000000..7091e4e --- /dev/null +++ b/test/asobi_body_cap_plugin_tests.erl @@ -0,0 +1,79 @@ +-module(asobi_body_cap_plugin_tests). +-include_lib("eunit/include/eunit.hrl"). + +%% H2 (2026-05-19): the body cap plugin must short-circuit oversized requests +%% before any body bytes are buffered. We meck cowboy_req so the test does not +%% need a real socket — the plugin should only call has_body/1, body_length/1 +%% and (on reject) reply/4. + +%% Cast a fake req map through dynamic() so eqwalizer accepts it as +%% cowboy_req:req() for the duration of the test. The plugin only touches the +%% three meck'd cowboy_req accessors above, so the underlying shape is fine. +-spec fake_req(map()) -> dynamic(). +fake_req(M) -> M. + +body_cap_test_() -> + {foreach, fun setup/0, fun teardown/1, [ + fun no_body_passes/0, + fun small_body_passes/0, + fun oversized_body_rejected_413/0, + fun chunked_without_length_rejected_411/0, + fun chunked_allowed_when_opt_off/0 + ]}. + +setup() -> + meck:new(cowboy_req, [no_link, passthrough]), + meck:expect(cowboy_req, reply, fun(Status, _Hdrs, _Body, Req) -> + Req#{reply_status => Status} + end), + ok. + +teardown(_) -> + meck:unload(cowboy_req), + ok. + +no_body_passes() -> + meck:expect(cowboy_req, has_body, fun(_) -> false end), + Req = fake_req(#{method => ~"GET"}), + ?assertMatch( + {ok, _, undefined}, + asobi_body_cap_plugin:pre_request(Req, #{}, #{}, undefined) + ). + +small_body_passes() -> + meck:expect(cowboy_req, has_body, fun(_) -> true end), + meck:expect(cowboy_req, body_length, fun(_) -> 1024 end), + Req = fake_req(#{method => ~"POST"}), + ?assertMatch( + {ok, _, undefined}, + asobi_body_cap_plugin:pre_request(Req, #{}, #{max_body => 1048576}, undefined) + ). + +oversized_body_rejected_413() -> + meck:expect(cowboy_req, has_body, fun(_) -> true end), + meck:expect(cowboy_req, body_length, fun(_) -> 2 * 1048576 end), + Req = fake_req(#{method => ~"POST"}), + {break, ReplyReq, undefined} = asobi_body_cap_plugin:pre_request( + Req, #{}, #{max_body => 1048576}, undefined + ), + ?assertEqual(413, maps:get(reply_status, ReplyReq)). + +chunked_without_length_rejected_411() -> + meck:expect(cowboy_req, has_body, fun(_) -> true end), + meck:expect(cowboy_req, body_length, fun(_) -> undefined end), + Req = fake_req(#{method => ~"POST"}), + {break, ReplyReq, undefined} = asobi_body_cap_plugin:pre_request( + Req, #{}, #{require_content_length => true}, undefined + ), + ?assertEqual(411, maps:get(reply_status, ReplyReq)). + +chunked_allowed_when_opt_off() -> + meck:expect(cowboy_req, has_body, fun(_) -> true end), + meck:expect(cowboy_req, body_length, fun(_) -> undefined end), + Req = fake_req(#{method => ~"POST"}), + ?assertMatch( + {ok, _, undefined}, + asobi_body_cap_plugin:pre_request( + Req, #{}, #{require_content_length => false}, undefined + ) + ). diff --git a/test/asobi_chat_acl_tests.erl b/test/asobi_chat_acl_tests.erl new file mode 100644 index 0000000..8855c22 --- /dev/null +++ b/test/asobi_chat_acl_tests.erl @@ -0,0 +1,19 @@ +-module(asobi_chat_acl_tests). +-include_lib("eunit/include/eunit.hrl"). + +%% H1 (2026-05-19): chat ACL must reject third parties on DMs. World and +%% group channels require live processes / DB and are exercised via +%% asobi_chat_SUITE; here we cover the deterministic dm path that the WS +%% handler now gates `chat.join` and `chat.send` through. + +dm_member_authorized_test() -> + ?assert(asobi_chat_acl:authorized(~"dm:alice:bob", ~"alice")), + ?assert(asobi_chat_acl:authorized(~"dm:alice:bob", ~"bob")). + +dm_third_party_rejected_test() -> + ?assertNot(asobi_chat_acl:authorized(~"dm:alice:bob", ~"eve")), + ?assertNot(asobi_chat_acl:authorized(~"dm:alice:bob", ~"")). + +dm_substring_does_not_grant_access_test() -> + ?assertNot(asobi_chat_acl:authorized(~"dm:alice:bob", ~"ali")), + ?assertNot(asobi_chat_acl:authorized(~"dm:alice:bob", ~"alicee")). diff --git a/test/asobi_world_lobby_cache_tests.erl b/test/asobi_world_lobby_cache_tests.erl new file mode 100644 index 0000000..fdbca27 --- /dev/null +++ b/test/asobi_world_lobby_cache_tests.erl @@ -0,0 +1,60 @@ +-module(asobi_world_lobby_cache_tests). +-include_lib("eunit/include/eunit.hrl"). + +%% H3 (2026-05-19): list_worlds_cached/1 must return cached entries within +%% the TTL window so a flood of WS world.list does not fan out to N +%% synchronous gen_server:calls per request. We poke ETS directly so the +%% test does not need running world processes. + +-define(TAB, asobi_world_lobby_cache). +-define(TTL_MS, 500). + +cache_test_() -> + {foreach, fun setup/0, fun teardown/1, [ + fun returns_cached_entry_within_ttl/0, + fun cache_keyed_per_filter/0, + fun ignores_expired_entry/0 + ]}. + +setup() -> + %% Make sure no stale table exists from a prior test process. Catch + %% badarg if it does not — owner is whichever test created it first. + catch ets:delete(?TAB), + ets:new(?TAB, [set, public, named_table, {read_concurrency, true}]), + ok. + +teardown(_) -> + catch ets:delete(?TAB), + ok. + +returns_cached_entry_within_ttl() -> + %% Pre-seed cache with a fake world so we can prove it is being read + %% rather than recomputed from pg. + Fake = #{world_id => ~"cached-world-id", mode => ~"barrow"}, + Now = erlang:monotonic_time(millisecond), + ets:insert(?TAB, {#{}, [Fake], Now + ?TTL_MS}), + ?assertEqual([Fake], asobi_world_lobby:list_worlds_cached(#{})). + +cache_keyed_per_filter() -> + Now = erlang:monotonic_time(millisecond), + A = #{world_id => ~"a", mode => ~"barrow"}, + B = #{world_id => ~"b", mode => ~"corsairs"}, + ets:insert(?TAB, {#{mode => ~"barrow"}, [A], Now + ?TTL_MS}), + ets:insert(?TAB, {#{mode => ~"corsairs"}, [B], Now + ?TTL_MS}), + ?assertEqual([A], asobi_world_lobby:list_worlds_cached(#{mode => ~"barrow"})), + ?assertEqual([B], asobi_world_lobby:list_worlds_cached(#{mode => ~"corsairs"})). + +ignores_expired_entry() -> + %% Expired entries must be treated as a miss. Without pg up the resulting + %% recompute will fail, so we only check that lookup classifies expired + %% as miss by ensuring no value comes back from a separate helper. + Fake = #{world_id => ~"stale", mode => ~"barrow"}, + Past = erlang:monotonic_time(millisecond) - 1, + ets:insert(?TAB, {#{}, [Fake], Past}), + %% Direct ETS check: the stale entry is still physically present. + %% list_worlds_cached would do the recompute (and crash without pg); + %% we assert the cache_lookup classification by inspecting the row's + %% expiry against "now". + [{_K, _W, Expires}] = ets:lookup(?TAB, #{}), + Now = erlang:monotonic_time(millisecond), + ?assert(Expires =< Now). diff --git a/test/asobi_world_lobby_ws_SUITE.erl b/test/asobi_world_lobby_ws_SUITE.erl index 3cd9247..bfb5205 100644 --- a/test/asobi_world_lobby_ws_SUITE.erl +++ b/test/asobi_world_lobby_ws_SUITE.erl @@ -403,9 +403,12 @@ recv_p1_update_with_x(PlayerId, X, Conn) -> fun(M) -> maps:get(~"type", M, undefined) =:= ~"world.tick" andalso lists:any( - fun(U) -> - maps:get(~"id", U, undefined) =:= PlayerId andalso - maps:get(~"x", U, undefined) =:= X + fun + (U) when is_map(U) -> + maps:get(~"id", U, undefined) =:= PlayerId andalso + maps:get(~"x", U, undefined) =:= X; + (_) -> + false end, maps:get(~"updates", maps:get(~"payload", M), []) ) From bb245984dc335a83a346b46522ab4abcada60c6b Mon Sep 17 00:00:00 2001 From: Daniel Widgren Date: Wed, 15 Jul 2026 05:52:13 +0200 Subject: [PATCH 2/2] 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). --- src/controllers/asobi_world_controller.erl | 7 +- src/plugins/asobi_body_cap_plugin.erl | 13 +- src/world/asobi_world_lobby.erl | 45 ++++--- src/world/asobi_world_lobby_server.erl | 27 +++- test/asobi_body_cap_plugin_tests.erl | 4 +- test/asobi_chat_ws_SUITE.erl | 136 +++++++++++++++++++++ test/asobi_world_lobby_cache_tests.erl | 71 ++++++++--- 7 files changed, 252 insertions(+), 51 deletions(-) create mode 100644 test/asobi_chat_ws_SUITE.erl diff --git a/src/controllers/asobi_world_controller.erl b/src/controllers/asobi_world_controller.erl index 6925ff9..2ca64d5 100644 --- a/src/controllers/asobi_world_controller.erl +++ b/src/controllers/asobi_world_controller.erl @@ -48,10 +48,13 @@ create(_Req) -> -spec build_filters(map()) -> map(). build_filters(QS) -> F0 = #{}, + %% Bound `mode` to the same 64-byte cap the WS path enforces + %% (asobi_ws_handler:build_world_filters/1); a longer value matches no + %% registered mode, so drop the filter rather than scan with it. F1 = case maps:get(~"mode", QS, undefined) of - undefined -> F0; - Mode -> F0#{mode => Mode} + Mode when is_binary(Mode), byte_size(Mode) =< 64 -> F0#{mode => Mode}; + _ -> F0 end, case maps:get(~"has_capacity", QS, undefined) of ~"true" -> F1#{has_capacity => true}; diff --git a/src/plugins/asobi_body_cap_plugin.erl b/src/plugins/asobi_body_cap_plugin.erl index d753f10..c82c1b1 100644 --- a/src/plugins/asobi_body_cap_plugin.erl +++ b/src/plugins/asobi_body_cap_plugin.erl @@ -20,7 +20,7 @@ Options: -define(DEFAULT_MAX_BODY, 1048576). -spec pre_request(cowboy_req:req(), map(), map(), term()) -> - {ok, cowboy_req:req(), term()} | {break, cowboy_req:req(), term()}. + {ok, cowboy_req:req(), term()} | {stop, cowboy_req:req(), term()}. pre_request(Req, _Env, Options, State) -> Max = maps:get(max_body, Options, ?DEFAULT_MAX_BODY), RequireCL = maps:get(require_content_length, Options, true), @@ -51,7 +51,7 @@ needs_check(Req) -> cowboy_req:has_body(Req). -spec check_size(cowboy_req:req(), non_neg_integer(), boolean(), term()) -> - {ok, cowboy_req:req(), term()} | {break, cowboy_req:req(), term()}. + {ok, cowboy_req:req(), term()} | {stop, cowboy_req:req(), term()}. check_size(Req, Max, RequireCL, State) -> case cowboy_req:body_length(Req) of undefined when RequireCL -> @@ -64,8 +64,13 @@ check_size(Req, Max, RequireCL, State) -> {ok, Req, State} end. +%% We send the response ourselves, then return {stop, Req, State}. nova maps a +%% pre_request {break, ...} to a 2-tuple {ok, Req} that cowboy's stream handler +%% has no clause for (case_clause -> request-process crash + crash report on +%% every rejection - log/CPU amplification on the path meant to reduce DoS). +%% {stop, ...} maps to {stop, Req}, which cowboy handles cleanly. -spec reject(integer(), binary(), cowboy_req:req(), term()) -> - {break, cowboy_req:req(), term()}. + {stop, cowboy_req:req(), term()}. reject(Status, Reason, Req, State) -> Body = json:encode(#{~"error" => Reason}), Req1 = cowboy_req:reply( @@ -74,4 +79,4 @@ reject(Status, Reason, Req, State) -> Body, Req ), - {break, Req1, State}. + {stop, Req1, State}. diff --git a/src/world/asobi_world_lobby.erl b/src/world/asobi_world_lobby.erl index c581059..93d4b53 100644 --- a/src/world/asobi_world_lobby.erl +++ b/src/world/asobi_world_lobby.erl @@ -47,10 +47,17 @@ list_worlds(Filters) -> H3 (2026-05-19): cached variant of `list_worlds/1` for request paths that do not need a fresh enumeration on every call. Each `list_worlds/1` call issues one synchronous `gen_server:call` per running world (`get_info/1`); -WS `world.list` at 60 msg/sec × 1000 worlds = 60k calls/sec/attacker. The -cache (500 ms TTL, populated lazily, backed by the ETS table owned by +WS `world.list` at 60 msg/sec x 1000 worlds = 60k calls/sec/attacker. The +cache (500 ms TTL, backed by the ETS table owned by `asobi_world_lobby_server`) absorbs that fan-out without changing the serialization story for `find_or_create_unsafe` which stays uncached. + +The cache key is the `has_capacity` boolean only, never the raw filter +map: `mode` is attacker-controlled and unbounded, so keying on it would +let a client cycle distinct modes to force a miss on every request +(defeating the cache) and grow the table without bound. Instead the full +enumeration is cached under at most two keys and `mode` is applied +in-memory on the cached list. """. -spec list_worlds_cached() -> [map()]. list_worlds_cached() -> @@ -58,18 +65,25 @@ list_worlds_cached() -> -spec list_worlds_cached(map()) -> [map()]. list_worlds_cached(Filters) -> - Key = Filters, + HasCapacity = maps:get(has_capacity, Filters, false), Now = erlang:monotonic_time(millisecond), - case cache_lookup(Key, Now) of - {hit, Worlds} -> - Worlds; - miss -> - Worlds = list_worlds(Filters), - cache_put(Key, Worlds, Now), - Worlds + All = + case cache_lookup(HasCapacity, Now) of + {hit, Worlds} -> + Worlds; + miss -> + Worlds = list_worlds(#{has_capacity => HasCapacity}), + asobi_world_lobby_server:cache_worlds( + HasCapacity, Worlds, Now + ?LIST_CACHE_TTL_MS + ), + Worlds + end, + case maps:get(mode, Filters, undefined) of + undefined -> All; + Mode -> [W || W <- All, maps:get(mode, W, undefined) =:= Mode] end. --spec cache_lookup(map(), integer()) -> {hit, [map()]} | miss. +-spec cache_lookup(boolean(), integer()) -> {hit, [map()]} | miss. cache_lookup(Key, Now) -> try ets:lookup(?LIST_CACHE_TAB, Key) of [{_, Worlds, ExpiresAt}] when ExpiresAt > Now -> {hit, Worlds}; @@ -78,15 +92,6 @@ cache_lookup(Key, Now) -> error:badarg -> miss end. --spec cache_put(map(), [map()], integer()) -> ok. -cache_put(Key, Worlds, Now) -> - try - ets:insert(?LIST_CACHE_TAB, {Key, Worlds, Now + ?LIST_CACHE_TTL_MS}), - ok - catch - error:badarg -> ok - end. - -doc """ Find a running world with capacity for the given mode, or create one. diff --git a/src/world/asobi_world_lobby_server.erl b/src/world/asobi_world_lobby_server.erl index b7e37a3..c576264 100644 --- a/src/world/asobi_world_lobby_server.erl +++ b/src/world/asobi_world_lobby_server.erl @@ -17,10 +17,14 @@ caller sees the world the first one just spawned. Calls are sequential, but `create_world/1` is the slowest step and takes <100ms in practice; for the small number of distinct modes a typical deployment supports, the queue stays empty. + +It also owns the protected ETS table backing +`asobi_world_lobby:list_worlds_cached/1`. Callers read the table +directly (protected reads); only this server writes, via `cache_worlds/3`. """. -behaviour(gen_server). --export([start_link/0, find_or_create/1, find_or_create/2]). +-export([start_link/0, find_or_create/1, find_or_create/2, cache_worlds/3]). -export([init/1, handle_call/3, handle_cast/2]). -define(CALL_TIMEOUT, 30000). @@ -48,6 +52,15 @@ find_or_create(Mode, PlayerId) -> Other -> {error, {unexpected_reply, Other}} end. +-doc """ +Store a computed world list against a bounded key (`has_capacity` +boolean) with an absolute expiry. Async: the caller keeps the value it +already computed; this only seeds the shared cache for the next reader. +""". +-spec cache_worlds(boolean(), [map()], integer()) -> ok. +cache_worlds(HasCapacity, Worlds, ExpiresAt) when is_boolean(HasCapacity) -> + gen_server:cast(?MODULE, {cache_worlds, HasCapacity, Worlds, ExpiresAt}). + %%-------------------------------------------------------------------- %% gen_server %%-------------------------------------------------------------------- @@ -55,12 +68,13 @@ find_or_create(Mode, PlayerId) -> -spec init([]) -> {ok, #{}}. init([]) -> %% H3 (2026-05-19): ETS-backed TTL cache for asobi_world_lobby:list_worlds/1. - %% Public read so callers do not have to round-trip this server; only the - %% server creates it so it survives crashes of any single caller. + %% Protected so any caller can read without a round-trip, but only this + %% server writes it (no in-node process can poison the shared list). Survives + %% crashes of any single caller because the server owns it. case ets:info(?LIST_CACHE_TAB, name) of undefined -> ?LIST_CACHE_TAB = ets:new(?LIST_CACHE_TAB, [ - set, public, named_table, {read_concurrency, true}, {write_concurrency, true} + set, protected, named_table, {read_concurrency, true} ]), ok; _ -> @@ -79,5 +93,10 @@ handle_call(_Other, _From, State) -> {reply, {error, unknown_request}, State}. -spec handle_cast(term(), map()) -> {noreply, map()}. +handle_cast({cache_worlds, HasCapacity, Worlds, ExpiresAt}, State) when + is_boolean(HasCapacity), is_list(Worlds), is_integer(ExpiresAt) +-> + ets:insert(?LIST_CACHE_TAB, {HasCapacity, Worlds, ExpiresAt}), + {noreply, State}; handle_cast(_Msg, State) -> {noreply, State}. diff --git a/test/asobi_body_cap_plugin_tests.erl b/test/asobi_body_cap_plugin_tests.erl index 7091e4e..0837920 100644 --- a/test/asobi_body_cap_plugin_tests.erl +++ b/test/asobi_body_cap_plugin_tests.erl @@ -53,7 +53,7 @@ oversized_body_rejected_413() -> meck:expect(cowboy_req, has_body, fun(_) -> true end), meck:expect(cowboy_req, body_length, fun(_) -> 2 * 1048576 end), Req = fake_req(#{method => ~"POST"}), - {break, ReplyReq, undefined} = asobi_body_cap_plugin:pre_request( + {stop, ReplyReq, undefined} = asobi_body_cap_plugin:pre_request( Req, #{}, #{max_body => 1048576}, undefined ), ?assertEqual(413, maps:get(reply_status, ReplyReq)). @@ -62,7 +62,7 @@ chunked_without_length_rejected_411() -> meck:expect(cowboy_req, has_body, fun(_) -> true end), meck:expect(cowboy_req, body_length, fun(_) -> undefined end), Req = fake_req(#{method => ~"POST"}), - {break, ReplyReq, undefined} = asobi_body_cap_plugin:pre_request( + {stop, ReplyReq, undefined} = asobi_body_cap_plugin:pre_request( Req, #{}, #{require_content_length => true}, undefined ), ?assertEqual(411, maps:get(reply_status, ReplyReq)). diff --git a/test/asobi_chat_ws_SUITE.erl b/test/asobi_chat_ws_SUITE.erl new file mode 100644 index 0000000..d4b6599 --- /dev/null +++ b/test/asobi_chat_ws_SUITE.erl @@ -0,0 +1,136 @@ +-module(asobi_chat_ws_SUITE). +-moduledoc """ +H1 (2026-05-19): the WS chat entrypoints must enforce +`asobi_chat_acl:authorized/2`. A third party must not be able to join or +send to a DM channel `dm::` it is not a party to; the two members +must be able to. Covers the wiring, not just the predicate (which is unit +tested in asobi_chat_acl_tests). +""". + +-include_lib("nova_test/include/nova_test.hrl"). + +-export([all/0, init_per_suite/1, end_per_suite/1]). +-export([ + dm_third_party_join_rejected/1, + dm_member_join_allowed/1, + dm_third_party_send_rejected/1 +]). + +all() -> + [ + dm_third_party_join_rejected, + dm_member_join_allowed, + dm_third_party_send_rejected + ]. + +init_per_suite(Config) -> + asobi_test_helpers:start(Config). + +end_per_suite(_Config) -> + ok. + +dm_third_party_join_rejected(Config) -> + {Alice, _} = register_player(~"alice", Config), + {Bob, _} = register_player(~"bob", Config), + {_Eve, EveTok} = register_player(~"eve", Config), + Conn = ws_connect_authed(EveTok, Config), + Channel = <<"dm:", Alice/binary, ":", Bob/binary>>, + ok = nova_test_ws:send_json( + #{ + ~"type" => ~"chat.join", + ~"cid" => ~"j1", + ~"payload" => #{~"channel_id" => Channel} + }, + Conn + ), + {ok, Reply} = recv_until(fun(M) -> maps:get(~"cid", M, undefined) =:= ~"j1" end, Conn), + nova_test_ws:close(Conn), + ?assertEqual(~"error", maps:get(~"type", Reply)), + ?assertEqual(~"not_authorized", maps:get(~"reason", maps:get(~"payload", Reply))), + Config. + +dm_member_join_allowed(Config) -> + {Alice, AliceTok} = register_player(~"alice2", Config), + {Bob, _} = register_player(~"bob2", Config), + Conn = ws_connect_authed(AliceTok, Config), + Channel = <<"dm:", Alice/binary, ":", Bob/binary>>, + ok = nova_test_ws:send_json( + #{ + ~"type" => ~"chat.join", + ~"cid" => ~"j2", + ~"payload" => #{~"channel_id" => Channel} + }, + Conn + ), + {ok, Reply} = recv_until(fun(M) -> maps:get(~"cid", M, undefined) =:= ~"j2" end, Conn), + nova_test_ws:close(Conn), + ?assertEqual(~"chat.joined", maps:get(~"type", Reply)), + Config. + +dm_third_party_send_rejected(Config) -> + {Alice, _} = register_player(~"alice3", Config), + {Bob, _} = register_player(~"bob3", Config), + {_Eve, EveTok} = register_player(~"eve3", Config), + Conn = ws_connect_authed(EveTok, Config), + Channel = <<"dm:", Alice/binary, ":", Bob/binary>>, + ok = nova_test_ws:send_json( + #{ + ~"type" => ~"chat.send", + ~"cid" => ~"s1", + ~"payload" => #{~"channel_id" => Channel, ~"content" => ~"leak?"} + }, + Conn + ), + {ok, Reply} = recv_until(fun(M) -> maps:get(~"cid", M, undefined) =:= ~"s1" end, Conn), + nova_test_ws:close(Conn), + ?assertEqual(~"error", maps:get(~"type", Reply)), + ?assertEqual(~"not_authorized", maps:get(~"reason", maps:get(~"payload", Reply))), + Config. + +%% --- helpers (mirrors asobi_world_lobby_ws_SUITE) --- + +register_player(Suffix, Config) -> + Username = unique_name(Suffix), + {ok, Resp} = nova_test:post( + "/api/v1/auth/register", + #{json => #{~"username" => Username, ~"password" => ~"testpass123"}}, + Config + ), + #{~"player_id" := PlayerId, ~"access_token" := Token} = nova_test:json(Resp), + {PlayerId, Token}. + +unique_name(Suffix) -> + N = integer_to_binary(erlang:unique_integer([positive])), + <<"chatprobe_", N/binary, "_", Suffix/binary>>. + +ws_connect_authed(Token, Config) -> + {ok, Conn} = nova_test_ws:connect("/ws", Config), + ok = nova_test_ws:send_json( + #{ + ~"type" => ~"session.connect", + ~"cid" => ~"sess", + ~"payload" => #{~"token" => Token} + }, + Conn + ), + {ok, _} = recv_until( + fun(M) -> maps:get(~"type", M, undefined) =:= ~"session.connected" end, + Conn + ), + Conn. + +recv_until(Pred, Conn) -> + recv_until(Pred, Conn, 50). + +recv_until(_Pred, _Conn, 0) -> + {error, predicate_not_matched}; +recv_until(Pred, Conn, N) -> + case nova_test_ws:recv_json(Conn) of + {ok, Msg} -> + case Pred(Msg) of + true -> {ok, Msg}; + false -> recv_until(Pred, Conn, N - 1) + end; + {error, _} = Err -> + Err + end. diff --git a/test/asobi_world_lobby_cache_tests.erl b/test/asobi_world_lobby_cache_tests.erl index fdbca27..a3522d3 100644 --- a/test/asobi_world_lobby_cache_tests.erl +++ b/test/asobi_world_lobby_cache_tests.erl @@ -5,6 +5,11 @@ %% the TTL window so a flood of WS world.list does not fan out to N %% synchronous gen_server:calls per request. We poke ETS directly so the %% test does not need running world processes. +%% +%% The cache is keyed on the has_capacity boolean only (at most two rows); +%% mode is applied in-memory on the cached list. Keying on the raw filter +%% map would let a client cycle distinct modes to force a miss on every +%% request and grow the table without bound. -define(TAB, asobi_world_lobby_cache). -define(TTL_MS, 500). @@ -12,49 +17,77 @@ cache_test_() -> {foreach, fun setup/0, fun teardown/1, [ fun returns_cached_entry_within_ttl/0, - fun cache_keyed_per_filter/0, + fun mode_filtered_in_memory/0, + fun distinct_modes_do_not_grow_table/0, + fun has_capacity_keyed_separately/0, fun ignores_expired_entry/0 ]}. setup() -> - %% Make sure no stale table exists from a prior test process. Catch - %% badarg if it does not — owner is whichever test created it first. - catch ets:delete(?TAB), + delete_tab(), ets:new(?TAB, [set, public, named_table, {read_concurrency, true}]), ok. teardown(_) -> - catch ets:delete(?TAB), + delete_tab(), ok. +delete_tab() -> + case ets:whereis(?TAB) of + undefined -> ok; + _ -> ets:delete(?TAB) + end. + returns_cached_entry_within_ttl() -> %% Pre-seed cache with a fake world so we can prove it is being read - %% rather than recomputed from pg. + %% rather than recomputed from pg. Default key is has_capacity=false. Fake = #{world_id => ~"cached-world-id", mode => ~"barrow"}, Now = erlang:monotonic_time(millisecond), - ets:insert(?TAB, {#{}, [Fake], Now + ?TTL_MS}), + ets:insert(?TAB, {false, [Fake], Now + ?TTL_MS}), ?assertEqual([Fake], asobi_world_lobby:list_worlds_cached(#{})). -cache_keyed_per_filter() -> +mode_filtered_in_memory() -> + %% One cached row (key=false) holds all worlds; mode is applied in-memory, + %% not as a cache key. Now = erlang:monotonic_time(millisecond), A = #{world_id => ~"a", mode => ~"barrow"}, B = #{world_id => ~"b", mode => ~"corsairs"}, - ets:insert(?TAB, {#{mode => ~"barrow"}, [A], Now + ?TTL_MS}), - ets:insert(?TAB, {#{mode => ~"corsairs"}, [B], Now + ?TTL_MS}), + ets:insert(?TAB, {false, [A, B], Now + ?TTL_MS}), ?assertEqual([A], asobi_world_lobby:list_worlds_cached(#{mode => ~"barrow"})), - ?assertEqual([B], asobi_world_lobby:list_worlds_cached(#{mode => ~"corsairs"})). + ?assertEqual([B], asobi_world_lobby:list_worlds_cached(#{mode => ~"corsairs"})), + ?assertEqual([], asobi_world_lobby:list_worlds_cached(#{mode => ~"nonexistent"})). + +distinct_modes_do_not_grow_table() -> + %% The DoS fix: cycling distinct (attacker-controlled) modes must all be + %% served from the single has_capacity=false row, never inserting new keys. + Now = erlang:monotonic_time(millisecond), + A = #{world_id => ~"a", mode => ~"barrow"}, + ets:insert(?TAB, {false, [A], Now + ?TTL_MS}), + lists:foreach( + fun(N) -> + Mode = integer_to_binary(N), + _ = asobi_world_lobby:list_worlds_cached(#{mode => Mode}) + end, + lists:seq(1, 50) + ), + ?assertEqual(1, ets:info(?TAB, size)). + +has_capacity_keyed_separately() -> + Now = erlang:monotonic_time(millisecond), + All = #{world_id => ~"all", mode => ~"barrow"}, + OpenOnly = #{world_id => ~"open", mode => ~"barrow"}, + ets:insert(?TAB, {false, [All], Now + ?TTL_MS}), + ets:insert(?TAB, {true, [OpenOnly], Now + ?TTL_MS}), + ?assertEqual([All], asobi_world_lobby:list_worlds_cached(#{})), + ?assertEqual([OpenOnly], asobi_world_lobby:list_worlds_cached(#{has_capacity => true})). ignores_expired_entry() -> %% Expired entries must be treated as a miss. Without pg up the resulting - %% recompute will fail, so we only check that lookup classifies expired - %% as miss by ensuring no value comes back from a separate helper. + %% recompute will fail, so we only assert the lookup classifies expired + %% as a miss by inspecting the row's expiry against "now". Fake = #{world_id => ~"stale", mode => ~"barrow"}, Past = erlang:monotonic_time(millisecond) - 1, - ets:insert(?TAB, {#{}, [Fake], Past}), - %% Direct ETS check: the stale entry is still physically present. - %% list_worlds_cached would do the recompute (and crash without pg); - %% we assert the cache_lookup classification by inspecting the row's - %% expiry against "now". - [{_K, _W, Expires}] = ets:lookup(?TAB, #{}), + ets:insert(?TAB, {false, [Fake], Past}), + [{_K, _W, Expires}] = ets:lookup(?TAB, false), Now = erlang:monotonic_time(millisecond), ?assert(Expires =< Now).