AMQP 1.0 binary parser: treat arrays with extra or missing input as fatal errors

With some input it is possible that the terminating clause will never
match.

While at it, consume binary input when parsing short form
primitives: null, true, false, as well as uint/ulong zero values.

Pair: @lhoguin.

Author: michaelklishin

deps/amqp10_common/src/amqp10_binary_parser.erl

@@ -31,15 +31,15 @@ parse_described(Bin) ->
     {Value, Rest2} = parse(Rest1),
     {{described, Descriptor, Value}, Rest2}.
 
-parse_primitive0(<<Type,Rest/binary>>) ->
+parse_primitive0(<<Type, Rest/binary>>) ->
     parse_primitive(Type, Rest).
 
 %% Constants
-parse_primitive(16#40, Rest) -> {null,       Rest};
-parse_primitive(16#41, Rest) -> {true,       Rest};
-parse_primitive(16#42, Rest) -> {false,      Rest};
-parse_primitive(16#43, Rest) -> {{uint, 0},  Rest};
-parse_primitive(16#44, Rest) -> {{ulong, 0}, Rest};
+parse_primitive(16#40, R) -> {null, R};
+parse_primitive(16#41, R) -> {true, R};
+parse_primitive(16#42, R) -> {false, R};
+parse_primitive(16#43, R) -> {{uint, 0}, R};
+parse_primitive(16#44, R) -> {{ulong, 0}, R};
 
 %% Fixed-widths. Most integral types have a compact encoding as a byte.
 parse_primitive(16#50, <<V:8/unsigned,  R/binary>>) -> {{ubyte, V},      R};
@@ -122,6 +122,14 @@ parse_compound1(Count, Bin, Acc) ->
     {Value, Rest} = parse(Bin),
     parse_compound1(Count - 1, Rest, [Value | Acc]).
 
+parse_array_primitive(16#40, <<_:8/unsigned, R/binary>>) -> {null, R};
+parse_array_primitive(16#41, <<_:8/unsigned, R/binary>>) -> {true, R};
+parse_array_primitive(16#42, <<_:8/unsigned, R/binary>>) -> {false, R};
+parse_array_primitive(16#43, <<_:8/unsigned, R/binary>>) -> {{uint, 0}, R};
+parse_array_primitive(16#44, <<_:8/unsigned, R/binary>>) -> {{ulong, 0}, R};
+parse_array_primitive(Marker, Data) ->
+    parse_primitive(Marker, Data).
+
 %% array structure is {array, Ctor, [Data]}
 %% e.g. {array, symbol, [<<"amqp:accepted:list">>]}
 parse_array(UnitSize, Bin) ->
@@ -141,8 +149,12 @@ parse_array1(Count, <<Type, ArrayBin/binary>>) ->
 
 parse_array2(0, Type, <<>>, Acc) ->
     {array, parse_constructor(Type), lists:reverse(Acc)};
+parse_array2(0, Type, Bin, Acc) ->
+    exit({failed_to_parse_array_extra_input_remaining, Type, Bin, Acc});
+parse_array2(Count, Type, <<>>, Acc) when Count > 0 ->
+    exit({failed_to_parse_array_insufficient_input, Type, Count, Acc});
 parse_array2(Count, Type, Bin, Acc) ->
-    {Value, Rest} = parse_primitive(Type, Bin),
+    {Value, Rest} = parse_array_primitive(Type, Bin),
     parse_array2(Count - 1, Type, Rest, [Value | Acc]).
 
 parse_constructor(16#a3) -> symbol;

deps/amqp10_common/test/binary_parser_SUITE.erl

@@ -0,0 +1,59 @@
+-module(binary_parser_SUITE).
+
+-compile(export_all).
+
+-export([
+         ]).
+
+-include_lib("common_test/include/ct.hrl").
+-include_lib("eunit/include/eunit.hrl").
+
+%%%===================================================================
+%%% Common Test callbacks
+%%%===================================================================
+
+all() ->
+    [
+     {group, tests}
+    ].
+
+
+all_tests() ->
+    [
+     array_with_extra_input
+    ].
+
+groups() ->
+    [
+     {tests, [], all_tests()}
+    ].
+
+init_per_suite(Config) ->
+    Config.
+
+end_per_suite(_Config) ->
+    ok.
+
+init_per_group(_Group, Config) ->
+    Config.
+
+end_per_group(_Group, _Config) ->
+    ok.
+
+init_per_testcase(_TestCase, Config) ->
+    Config.
+
+end_per_testcase(_TestCase, _Config) ->
+    ok.
+
+%%%===================================================================
+%%% Test cases
+%%%===================================================================
+
+array_with_extra_input(_Config) ->
+    Bin = <<83,16,192,85,10,177,0,0,0,1,48,161,12,114,97,98,98,105,116, 109,113,45,98,111,120,112,255,255,0,0,96,0,50,112,0,0,19,136,163,5,101,110,45,85,83,224,14,2,65,5,102,105,45,70,73,5,101,110,45,85,83,64,64,193,24,2,163,20,68,69,70,69,78,83,73,67,83, 46,84,69,83,84,46,83,85,73,84,69,65>>,
+    ?assertExit({failed_to_parse_array_extra_input_remaining,
+        %% element type, input, accumulated result
+        65, <<105,45,70,73,5,101,110,45,85,83>>, [true,true]},
+        amqp10_binary_parser:parse_all(Bin)),
+    ok.