Просмотр исходного кода

Fix some HPACK Huffman decoding failure cases

When EOS is found within the string the decoding must fail.
When the final padding is > 7 bit in length the decoding must
fail as well.
Loïc Hoguin 5 лет назад
Родитель
Сommit
4b369969ce
2 измененных файлов с 43 добавлено и 25 удалено
  1. 28 10
      src/cow_hpack.erl
  2. 15 15
      src/cow_hpack_dec_huffman_lookup.hrl

+ 28 - 10
src/cow_hpack.erl

@@ -209,9 +209,9 @@ dec_str(<<0:1, Length:7, Rest0/bits>>) ->
 	{Str, Rest};
 dec_str(<<1:1, 2#1111111:7, Rest0/bits>>) ->
 	{Length, Rest} = dec_big_int(Rest0, 127, 0),
-	dec_huffman(Rest, Length, 0, ok, <<>>);
+	dec_huffman(Rest, Length, 0, <<>>);
 dec_str(<<1:1, Length:7, Rest/bits>>) ->
-	dec_huffman(Rest, Length, 0, ok, <<>>).
+	dec_huffman(Rest, Length, 0, <<>>).
 
 %% We use a lookup table that allows us to benefit from
 %% the binary match context optimization. A more naive
@@ -221,21 +221,39 @@ dec_str(<<1:1, Length:7, Rest/bits>>) ->
 %%
 %% See cow_hpack_dec_huffman_lookup.hrl for more details.
 
-dec_huffman(<<A:4, B:4, R/bits>>, Len, Huff0, _, Acc) when Len > 0 ->
+dec_huffman(<<A:4, B:4, R/bits>>, Len, Huff0, Acc) when Len > 1 ->
 	{_, CharA, Huff1} = dec_huffman_lookup(Huff0, A),
-	{Result, CharB, Huff} = dec_huffman_lookup(Huff1, B),
+	{_, CharB, Huff} = dec_huffman_lookup(Huff1, B),
 	case {CharA, CharB} of
-		{undefined, undefined} -> dec_huffman(R, Len - 1, Huff, Result, Acc);
-		{CharA, undefined} -> dec_huffman(R, Len - 1, Huff, Result, <<Acc/binary, CharA>>);
-		{undefined, CharB} -> dec_huffman(R, Len - 1, Huff, Result, <<Acc/binary, CharB>>);
-		{CharA, CharB} -> dec_huffman(R, Len - 1, Huff, Result, <<Acc/binary, CharA, CharB>>)
+		{undefined, undefined} -> dec_huffman(R, Len - 1, Huff, Acc);
+		{CharA, undefined} -> dec_huffman(R, Len - 1, Huff, <<Acc/binary, CharA>>);
+		{undefined, CharB} -> dec_huffman(R, Len - 1, Huff, <<Acc/binary, CharB>>);
+		{CharA, CharB} -> dec_huffman(R, Len - 1, Huff, <<Acc/binary, CharA, CharB>>)
 	end;
-dec_huffman(Rest, 0, _, ok, Acc) ->
-	{Acc, Rest}.
+dec_huffman(<<A:4, B:4, Rest/bits>>, 1, Huff0, Acc) ->
+	{_, CharA, Huff} = dec_huffman_lookup(Huff0, A),
+	{ok, CharB, _} = dec_huffman_lookup(Huff, B),
+	case {CharA, CharB} of
+		%% {undefined, undefined} (> 7-bit final padding) is rejected with a crash.
+		{CharA, undefined} ->
+			{<<Acc/binary, CharA>>, Rest};
+		{undefined, CharB} ->
+			{<<Acc/binary, CharB>>, Rest};
+		_ ->
+			{<<Acc/binary, CharA, CharB>>, Rest}
+	end;
+%% Can only be reached when the string length to decode is 0.
+dec_huffman(Rest, 0, _, <<>>) ->
+	{<<>>, Rest}.
 
 -include("cow_hpack_dec_huffman_lookup.hrl").
 
 -ifdef(TEST).
+%% Test case extracted from h2spec.
+decode_reject_eos_test() ->
+	{'EXIT', _} = (catch decode(<<16#0085f2b24a84ff874951fffffffa7f:120>>)),
+	ok.
+
 req_decode_test() ->
 	%% First request (raw then huffman).
 	{Headers1, State1} = decode(<< 16#828684410f7777772e6578616d706c652e636f6d:160 >>),

+ 15 - 15
src/cow_hpack_dec_huffman_lookup.hrl

@@ -4049,7 +4049,7 @@ dec_huffman_lookup(16#fa, 16#b) -> {ok, 16#f9, 16#28};
 dec_huffman_lookup(16#fa, 16#c) -> {ok, 16#0a, 16#00};
 dec_huffman_lookup(16#fa, 16#d) -> {ok, 16#0d, 16#00};
 dec_huffman_lookup(16#fa, 16#e) -> {ok, 16#16, 16#00};
-dec_huffman_lookup(16#fa, 16#f) -> {more, undefined, 16#fa};
+dec_huffman_lookup(16#fa, 16#f) -> error;
 dec_huffman_lookup(16#fb, 16#0) -> {more, 16#7f, 16#03};
 dec_huffman_lookup(16#fb, 16#1) -> {more, 16#7f, 16#06};
 dec_huffman_lookup(16#fb, 16#2) -> {more, 16#7f, 16#0a};
@@ -4080,8 +4080,8 @@ dec_huffman_lookup(16#fc, 16#a) -> {more, 16#0d, 16#01};
 dec_huffman_lookup(16#fc, 16#b) -> {ok, 16#0d, 16#16};
 dec_huffman_lookup(16#fc, 16#c) -> {more, 16#16, 16#01};
 dec_huffman_lookup(16#fc, 16#d) -> {ok, 16#16, 16#16};
-dec_huffman_lookup(16#fc, 16#e) -> {more, undefined, 16#fc};
-dec_huffman_lookup(16#fc, 16#f) -> {more, undefined, 16#fc};
+dec_huffman_lookup(16#fc, 16#e) -> error;
+dec_huffman_lookup(16#fc, 16#f) -> error;
 dec_huffman_lookup(16#fd, 16#0) -> {more, 16#0a, 16#02};
 dec_huffman_lookup(16#fd, 16#1) -> {more, 16#0a, 16#09};
 dec_huffman_lookup(16#fd, 16#2) -> {more, 16#0a, 16#17};
@@ -4094,10 +4094,10 @@ dec_huffman_lookup(16#fd, 16#8) -> {more, 16#16, 16#02};
 dec_huffman_lookup(16#fd, 16#9) -> {more, 16#16, 16#09};
 dec_huffman_lookup(16#fd, 16#a) -> {more, 16#16, 16#17};
 dec_huffman_lookup(16#fd, 16#b) -> {ok, 16#16, 16#28};
-dec_huffman_lookup(16#fd, 16#c) -> {more, undefined, 16#fd};
-dec_huffman_lookup(16#fd, 16#d) -> {more, undefined, 16#fd};
-dec_huffman_lookup(16#fd, 16#e) -> {more, undefined, 16#fd};
-dec_huffman_lookup(16#fd, 16#f) -> {more, undefined, 16#fd};
+dec_huffman_lookup(16#fd, 16#c) -> error;
+dec_huffman_lookup(16#fd, 16#d) -> error;
+dec_huffman_lookup(16#fd, 16#e) -> error;
+dec_huffman_lookup(16#fd, 16#f) -> error;
 dec_huffman_lookup(16#fe, 16#0) -> {more, 16#0a, 16#03};
 dec_huffman_lookup(16#fe, 16#1) -> {more, 16#0a, 16#06};
 dec_huffman_lookup(16#fe, 16#2) -> {more, 16#0a, 16#0a};
@@ -4122,11 +4122,11 @@ dec_huffman_lookup(16#ff, 16#4) -> {more, 16#16, 16#18};
 dec_huffman_lookup(16#ff, 16#5) -> {more, 16#16, 16#1f};
 dec_huffman_lookup(16#ff, 16#6) -> {more, 16#16, 16#29};
 dec_huffman_lookup(16#ff, 16#7) -> {ok, 16#16, 16#38};
-dec_huffman_lookup(16#ff, 16#8) -> {more, undefined, 16#ff};
-dec_huffman_lookup(16#ff, 16#9) -> {more, undefined, 16#ff};
-dec_huffman_lookup(16#ff, 16#a) -> {more, undefined, 16#ff};
-dec_huffman_lookup(16#ff, 16#b) -> {more, undefined, 16#ff};
-dec_huffman_lookup(16#ff, 16#c) -> {more, undefined, 16#ff};
-dec_huffman_lookup(16#ff, 16#d) -> {more, undefined, 16#ff};
-dec_huffman_lookup(16#ff, 16#e) -> {more, undefined, 16#ff};
-dec_huffman_lookup(16#ff, 16#f) -> {more, undefined, 16#ff}.
+dec_huffman_lookup(16#ff, 16#8) -> error;
+dec_huffman_lookup(16#ff, 16#9) -> error;
+dec_huffman_lookup(16#ff, 16#a) -> error;
+dec_huffman_lookup(16#ff, 16#b) -> error;
+dec_huffman_lookup(16#ff, 16#c) -> error;
+dec_huffman_lookup(16#ff, 16#d) -> error;
+dec_huffman_lookup(16#ff, 16#e) -> error;
+dec_huffman_lookup(16#ff, 16#f) -> error.