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.

2 files changed, 78 insertions, 7 deletions

diff --git a/deps/amqp10_common/src/amqp10_binary_parser.erl b/deps/amqp10_common/src/amqp10_binary_parser.erl
index b936f9f4ca..2ea5774c01 100644
--- a/deps/amqp10_common/src/amqp10_binary_parser.erl
+++ b/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;
diff --git a/deps/amqp10_common/test/binary_parser_SUITE.erl b/deps/amqp10_common/test/binary_parser_SUITE.erl
new file mode 100644
index 0000000000..b391cdb100
--- /dev/null
+++ b/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.