fix(security): H1-H3 High findings from 2026-05-19 audit (supersedes #125)

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

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

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>:<B>                     — A and B are the only allowed readers
-%%   world:<WorldId>                — must be currently joined to the world
-%%   zone:<WorldId>:<X>,<Y>         — must be currently joined to the world
-%%   prox:<WorldId>:<X>,<Y>         — must be currently joined to the world
-%%   <anything else>                — 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.

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}.
@@ -46,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};

src/plugins/asobi_body_cap_plugin.erl

@@ -0,0 +1,82 @@
+-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()} | {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),
+    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()} | {stop, 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.
+
+%% 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()) ->
+    {stop, 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
+    ),
+    {stop, Req1, State}.

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>:<B>                 - A and B are the only allowed readers
+  world:<WorldId>            - must currently be joined to the world
+  zone:<WorldId>:<X>,<Y>     - must currently be joined to the world
+  prox:<WorldId>:<X>,<Y>     - must currently be joined to the world
+  <anything else>            - 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.

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,55 @@ 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 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() ->
+    list_worlds_cached(#{}).
+
+-spec list_worlds_cached(map()) -> [map()].
+list_worlds_cached(Filters) ->
+    HasCapacity = maps:get(has_capacity, Filters, false),
+    Now = erlang:monotonic_time(millisecond),
+    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(boolean(), 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.
+
 -doc """
 Find a running world with capacity for the given mode, or create one.