From 9e5a868bf1c98548b955f8b720995b3f91153f5a Mon Sep 17 00:00:00 2001
From: Shawn <506895667@qq.com>
Date: Wed, 10 Mar 2021 16:49:58 +0800
Subject: [PATCH] fix(emqx): validate mqtt malformed variable byte integer

---
 src/emqx.appup.src        | 12 ++++++++++++
 src/emqx_frame.erl        |  3 +++
 test/emqx_frame_SUITE.erl |  9 ++++++++-
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/src/emqx.appup.src b/src/emqx.appup.src
index 8214b2e7f..bb3809156 100644
--- a/src/emqx.appup.src
+++ b/src/emqx.appup.src
@@ -8,16 +8,22 @@
     end,
 {VSN,
   [
+    {<<"4.2.[34567]">>, [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []}
+    ]},
     {"4.2.2", [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []},
       {load_module, emqx_metrics, brutal_purge, soft_purge, []}
     ]},
     {"4.2.1", [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []},
       {load_module, emqx_metrics, brutal_purge, soft_purge, []},
       {load_module, emqx_channel, brutal_purge, soft_purge, []},
       {load_module, emqx_mod_topic_metrics, brutal_purge, soft_purge, []},
       {load_module, emqx_json, brutal_purge, soft_purge, []}
     ]},
     {"4.2.0", [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []},
       {load_module, emqx_metrics, brutal_purge, soft_purge, []},
       {load_module, emqx_channel, brutal_purge, soft_purge, []},
       {load_module, emqx_mod_topic_metrics, brutal_purge, soft_purge, []},
@@ -30,16 +36,22 @@
     {<<".*">>, []}
   ],
   [
+    {<<"4.2.[34567]">>, [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []}
+    ]},
     {"4.2.2", [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []},
       {load_module, emqx_metrics, brutal_purge, soft_purge, []}
     ]},
     {"4.2.1", [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []},
       {load_module, emqx_metrics, brutal_purge, soft_purge, []},
       {load_module, emqx_channel, brutal_purge, soft_purge, []},
       {load_module, emqx_mod_topic_metrics, brutal_purge, soft_purge, []},
       {load_module, emqx_json, brutal_purge, soft_purge, []}
     ]},
     {"4.2.0", [
+      {load_module, emqx_frame, brutal_purge, soft_purge, []},
       {load_module, emqx_metrics, brutal_purge, soft_purge, []},
       {load_module, emqx_channel, brutal_purge, soft_purge, []},
       {load_module, emqx_mod_topic_metrics, brutal_purge, soft_purge, []},
diff --git a/src/emqx_frame.erl b/src/emqx_frame.erl
index 1c27548f7..21015d7bd 100644
--- a/src/emqx_frame.erl
+++ b/src/emqx_frame.erl
@@ -125,6 +125,9 @@ parse_remaining_len(<<0:8, Rest/binary>>, Header, 1, 0, Options) ->
 %% Match PUBACK, PUBREC, PUBREL, PUBCOMP, UNSUBACK...
 parse_remaining_len(<<0:1, 2:7, Rest/binary>>, Header, 1, 0, Options) ->
     parse_frame(Rest, Header, 2, Options);
+parse_remaining_len(<<1:1, _Len:7, _Rest/binary>>, _Header, Multiplier, _Value, _Options)
+         when Multiplier > 2097152 ->
+    error(malformed_variable_byte_integer);
 parse_remaining_len(<<1:1, Len:7, Rest/binary>>, Header, Multiplier, Value, Options) ->
     parse_remaining_len(Rest, Header, Multiplier * ?HIGHBIT, Value + Len * Multiplier, Options);
 parse_remaining_len(<<0:1, Len:7, Rest/binary>>, Header, Multiplier, Value,
diff --git a/test/emqx_frame_SUITE.erl b/test/emqx_frame_SUITE.erl
index 5d4e146db..aef88cf9f 100644
--- a/test/emqx_frame_SUITE.erl
+++ b/test/emqx_frame_SUITE.erl
@@ -42,7 +42,8 @@ all() ->
 groups() ->
     [{parse, [parallel],
       [t_parse_cont,
-       t_parse_frame_too_large
+       t_parse_frame_too_large,
+       t_parse_frame_malformed_variable_byte_integer
       ]},
      {connect, [parallel],
       [t_serialize_parse_v3_connect,
@@ -129,6 +130,12 @@ t_parse_frame_too_large(_) ->
     ?catch_error(frame_too_large, parse_serialize(Packet, #{max_size => 512})),
     ?assertEqual(Packet, parse_serialize(Packet, #{max_size => 2048, version => ?MQTT_PROTO_V4})).
 
+t_parse_frame_malformed_variable_byte_integer(_) ->
+    MalformedPayload = << <<16#80>> || _ <- lists:seq(1, 4) >>,
+    ParseState = emqx_frame:initial_parse_state(#{}),
+    ?catch_error(malformed_variable_byte_integer,
+        emqx_frame:parse(MalformedPayload, ParseState)).
+
 t_serialize_parse_v3_connect(_) ->
     Bin = <<16,37,0,6,77,81,73,115,100,112,3,2,0,60,0,23,109,111,115,
             113,112,117, 98,47,49,48,52,53,49,45,105,77,97,99,46,108,