Native STOMP: make the frame parser more robust

1. A negative content-length crashed the parser in `parse_known_body/4`.
2. A non-numeric content-length (e.g. "abc") crashed `integer_header/2`
    with a `badarg` exception
3. A content-length pointing past the NUL
   terminator resulted in a `badmatch`.

All three are pre-auth exceptions.

With these changes:

1. Negative values now return `{error, {invalid_content_length, N}}`
2. Non-numeric values are treated as absent
3. A missing NUL terminator returns `{error, missing_body_terminator}`

Author: michaelklishin

deps/rabbitmq_stomp/src/rabbit_stomp_frame.erl

@@ -364,6 +364,9 @@ parse_body(Content, #ps{cmd = Cmd, hdrs = Hdrs,
     case Cmd of
         'SEND' ->
             case integer_header(Frame, ?HEADER_CONTENT_LENGTH, unknown) of
+                ContentLength when is_integer(ContentLength),
+                                   ContentLength < 0 ->
+                    {error, {invalid_content_length, ContentLength}};
                 ContentLength when is_integer(ContentLength),
                                    ContentLength > MaxBodyLength ->
                     {error, {max_body_length, ContentLength}};
@@ -403,9 +406,13 @@ parse_known_body(Content, Frame, Chunks, Remaining) ->
     end.
 
 finish_body(Content, Frame, Chunks, Pos) ->
-    <<Chunk:Pos/binary, 0, Rest/binary>> = Content,
-    Body = finalize_chunk(Chunk, Chunks),
-    {ok, Frame#stomp_frame{body_iolist_rev = Body}, Rest}.
+    case Content of
+        <<Chunk:Pos/binary, 0, Rest/binary>> ->
+            Body = finalize_chunk(Chunk, Chunks),
+            {ok, Frame#stomp_frame{body_iolist_rev = Body}, Rest};
+        _ ->
+            {error, missing_body_terminator}
+    end.
 
 finalize_chunk(<<>>,  Chunks) -> Chunks;
 finalize_chunk(Chunk, Chunks) -> [Chunk | Chunks].
@@ -444,7 +451,10 @@ boolean_header(F, K, D) -> default_value(boolean_header(F, K), D).
 
 integer_header(F, Key) ->
     case header(F, Key) of
-        {ok, Str} -> {ok, binary_to_integer(string:trim(Str))};
+        {ok, Str} ->
+            try {ok, binary_to_integer(string:trim(Str))}
+            catch error:badarg -> not_found
+            end;
         not_found -> not_found
     end.
 

deps/rabbitmq_stomp/test/prop_frame_SUITE.erl

@@ -26,7 +26,10 @@ groups() ->
       [
        prop_within_limit_succeeds,
        prop_over_limit_fails,
-       prop_duplicates_do_not_exhaust_limit
+       prop_duplicates_do_not_exhaust_limit,
+       prop_negative_content_length_rejected,
+       prop_valid_content_length_round_trips,
+       prop_non_numeric_content_length_does_not_crash
       ]}].
 
 %% Any frame with up to LIMIT unique header names parses successfully.
@@ -65,6 +68,46 @@ prop_duplicates_do_not_exhaust_limit(_Config) ->
               end)
       end, [], 1000).
 
+%% Any negative content-length is rejected.
+prop_negative_content_length_rejected(_Config) ->
+    run_proper(
+      fun() ->
+          ?FORALL(N, range(-10000, -1),
+              begin
+                  {error, {invalid_content_length, N}} =
+                      parse(send_frame("content-length", integer_to_list(N), "x")),
+                  true
+              end)
+      end, [], 1000).
+
+%% A SEND frame with a matching content-length and body always parses.
+prop_valid_content_length_round_trips(_Config) ->
+    run_proper(
+      fun() ->
+          ?FORALL(Body, binary(),
+              begin
+                  Len = integer_to_list(byte_size(Body)),
+                  case parse(send_frame("content-length", Len, Body)) of
+                      {ok, #stomp_frame{command = 'SEND'}, _} -> true;
+                      {more, _} -> true
+                  end
+              end)
+      end, [], 1000).
+
+%% Non-numeric content-length values must never crash the parser.
+prop_non_numeric_content_length_does_not_crash(_Config) ->
+    run_proper(
+      fun() ->
+          ?FORALL(Junk, non_numeric_bin(),
+              begin
+                  case parse(send_frame("content-length", binary_to_list(Junk), "x")) of
+                      {ok, _, _} -> true;
+                      {more, _}  -> true;
+                      {error, _} -> true
+                  end
+              end)
+      end, [], 1000).
+
 %%-------------------------------------------------------------------
 
 unique_headers(N) ->
@@ -74,5 +117,18 @@ make_frame(Headers) ->
     HdrStr = lists:flatten([K ++ ":" ++ V ++ "\n" || {K, V} <- Headers]),
     iolist_to_binary(["CONNECT\n", HdrStr, "\n\0"]).
 
+send_frame(HdrName, HdrValue, Body) ->
+    iolist_to_binary(["SEND\ndestination:/queue/t\n",
+                      HdrName, ":", HdrValue, "\n\n",
+                      Body, "\0"]).
+
+%% Generator for binaries that are not valid integer strings.
+non_numeric_bin() ->
+    ?SUCHTHAT(Bin,
+              ?LET(Chars, list(range(0, 255)),
+                   list_to_binary(
+                     [C || C <- Chars, C =/= $\n, C =/= $\r, C =/= $:, C =/= $\\, C =/= 0])),
+              not is_integer(catch binary_to_integer(string:trim(Bin)))).
+
 parse(Bin) ->
     rabbit_stomp_frame:parse(Bin, rabbit_stomp_frame:initial_state()).

deps/rabbitmq_stomp/test/unit_content_length_SUITE.erl

@@ -0,0 +1,62 @@
+%% This Source Code Form is subject to the terms of the Mozilla Public
+%% License, v. 2.0. If a copy of the MPL was not distributed with this
+%% file, You can obtain one at https://mozilla.org/MPL/2.0/.
+%%
+%% Copyright (c) 2007-2026 Broadcom. All Rights Reserved. The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. All rights reserved.
+%%
+
+-module(unit_content_length_SUITE).
+
+-include_lib("eunit/include/eunit.hrl").
+-include("rabbit_stomp_frame.hrl").
+-compile(export_all).
+
+all() ->
+    [
+     negative_content_length_rejected,
+     non_numeric_content_length_treated_as_unknown,
+     zero_content_length_accepted,
+     valid_content_length_accepted,
+     missing_nul_terminator_rejected,
+     content_length_with_whitespace_accepted
+    ].
+
+negative_content_length_rejected(_) ->
+    ?assertMatch({error, {invalid_content_length, -1}},
+                 parse(send_frame([{"content-length", "-1"}], "hello"))).
+
+non_numeric_content_length_treated_as_unknown(_) ->
+    %% Non-numeric content-length is ignored; the parser falls back
+    %% to scanning for the NUL terminator.
+    {ok, Frame, _} = parse(send_frame([{"content-length", "abc"}], "hello")),
+    ?assertEqual('SEND', Frame#stomp_frame.command).
+
+zero_content_length_accepted(_) ->
+    {ok, Frame, _} = parse(send_frame([{"content-length", "0"}], "")),
+    ?assertEqual('SEND', Frame#stomp_frame.command),
+    ?assertEqual([], Frame#stomp_frame.body_iolist_rev).
+
+valid_content_length_accepted(_) ->
+    {ok, Frame, _} = parse(send_frame([{"content-length", "5"}], "hello")),
+    ?assertEqual('SEND', Frame#stomp_frame.command),
+    ?assertEqual([<<"hello">>], Frame#stomp_frame.body_iolist_rev).
+
+%% When content-length is set and the byte at that position is not NUL,
+%% the parser must return an error instead of crashing.
+missing_nul_terminator_rejected(_) ->
+    Bin = <<"SEND\ndestination:/queue/t\ncontent-length:3\n\nhelloX">>,
+    ?assertMatch({error, missing_body_terminator}, parse(Bin)).
+
+content_length_with_whitespace_accepted(_) ->
+    {ok, Frame, _} = parse(send_frame([{"content-length", " 5 "}], "hello")),
+    ?assertEqual('SEND', Frame#stomp_frame.command).
+
+%%-------------------------------------------------------------------
+
+send_frame(Headers, Body) ->
+    HdrStr = lists:flatten(
+               [K ++ ":" ++ V ++ "\n" || {K, V} <- [{"destination", "/queue/t"} | Headers]]),
+    iolist_to_binary(["SEND\n", HdrStr, "\n", Body, "\0"]).
+
+parse(Bin) ->
+    rabbit_stomp_frame:parse(Bin, rabbit_stomp_frame:initial_state()).