Merge branch 'bjorn/asn1/decoding-robustness/OTP-12145' into maint

* bjorn/asn1/decoding-robustness/OTP-12145:
  BER decoding: Improve error checking for indefinite length
  BER: Test decoding of indefinite lengths

3 files changed, 86 insertions, 22 deletions

diff --git a/lib/asn1/c_src/asn1_erl_nif.c b/lib/asn1/c_src/asn1_erl_nif.c
index 8a0e4b1cf0..53e3aa1678 100644
--- a/lib/asn1/c_src/asn1_erl_nif.c
+++ b/lib/asn1/c_src/asn1_erl_nif.c
@@ -941,16 +941,31 @@ static int ber_decode_value(ErlNifEnv* env, ERL_NIF_TERM *value, unsigned char *
     int maybe_ret;
     unsigned int len = 0;
     unsigned int lenoflen = 0;
-    int indef = 0;
     unsigned char *tmp_out_buff;
     ERL_NIF_TERM term = 0, curr_head = 0;
 
     if (((in_buf[*ib_index]) & 0x80) == ASN1_SHORT_DEFINITE_LENGTH) {
 	len = in_buf[*ib_index];
-    } else if (in_buf[*ib_index] == ASN1_INDEFINITE_LENGTH
-    )
-	indef = 1;
-    else /* long definite length */{
+    } else if (in_buf[*ib_index] == ASN1_INDEFINITE_LENGTH) {
+	(*ib_index)++;
+	curr_head = enif_make_list(env, 0);
+	if (*ib_index+1 >= in_buf_len) {
+	    return ASN1_INDEF_LEN_ERROR;
+	}
+	while (!(in_buf[*ib_index] == 0 && in_buf[*ib_index + 1] == 0)) {
+	    maybe_ret = ber_decode(env, &term, in_buf, ib_index, in_buf_len);
+	    if (maybe_ret <= ASN1_ERROR) {
+		return maybe_ret;
+	    }
+	    curr_head = enif_make_list_cell(env, term, curr_head);
+	    if (*ib_index+1 >= in_buf_len) {
+		return ASN1_INDEF_LEN_ERROR;
+	    }
+	}
+	enif_make_reverse_list(env, curr_head, value);
+	(*ib_index) += 2; /* skip the indefinite length end bytes */
+	return ASN1_OK;
+    } else /* long definite length */{
 	lenoflen = (in_buf[*ib_index] & 0x7f); /*length of length */
 	if (lenoflen > (in_buf_len - (*ib_index + 1)))
 	    return ASN1_LEN_ERROR;
@@ -965,23 +980,7 @@ static int ber_decode_value(ErlNifEnv* env, ERL_NIF_TERM *value, unsigned char *
     if (len > (in_buf_len - (*ib_index + 1)))
 	return ASN1_VALUE_ERROR;
     (*ib_index)++;
-    if (indef == 1) { /* in this case it is desireably to check that indefinite length
-     end bytes exist in inbuffer */
-	curr_head = enif_make_list(env, 0);
-	while (!(in_buf[*ib_index] == 0 && in_buf[*ib_index + 1] == 0)) {
-	    if (*ib_index >= in_buf_len)
-		return ASN1_INDEF_LEN_ERROR;
-
-	    if ((maybe_ret = ber_decode(env, &term, in_buf, ib_index, in_buf_len))
-		    <= ASN1_ERROR
-		    )
-		return maybe_ret;
-	    curr_head = enif_make_list_cell(env, term, curr_head);
-	}
-	enif_make_reverse_list(env, curr_head, value);
-	(*ib_index) += 2; /* skip the indefinite length end bytes */
-    } else if (form == ASN1_CONSTRUCTED)
-    {
+    if (form == ASN1_CONSTRUCTED) {
 	int end_index = *ib_index + len;
 	if (end_index > in_buf_len)
 	    return ASN1_LEN_ERROR;
diff --git a/lib/asn1/test/asn1_test_lib.erl b/lib/asn1/test/asn1_test_lib.erl
index 06e9b2c093..da07cd1118 100644
--- a/lib/asn1/test/asn1_test_lib.erl
+++ b/lib/asn1/test/asn1_test_lib.erl
@@ -112,6 +112,7 @@ roundtrip(Mod, Type, Value) ->
 roundtrip(Mod, Type, Value, ExpectedValue) ->
     {ok,Encoded} = Mod:encode(Type, Value),
     {ok,ExpectedValue} = Mod:decode(Type, Encoded),
+    test_ber_indefinite(Mod, Type, Encoded, ExpectedValue),
     ok.
 
 roundtrip_enc(Mod, Type, Value) ->
@@ -120,6 +121,7 @@ roundtrip_enc(Mod, Type, Value) ->
 roundtrip_enc(Mod, Type, Value, ExpectedValue) ->
     {ok,Encoded} = Mod:encode(Type, Value),
     {ok,ExpectedValue} = Mod:decode(Type, Encoded),
+    test_ber_indefinite(Mod, Type, Encoded, ExpectedValue),
     Encoded.
 
 %%%
@@ -129,3 +131,52 @@ roundtrip_enc(Mod, Type, Value, ExpectedValue) ->
 hex2num(C) when $0 =< C, C =< $9 -> C - $0;
 hex2num(C) when $A =< C, C =< $F -> C - $A + 10;
 hex2num(C) when $a =< C, C =< $f -> C - $a + 10.
+
+test_ber_indefinite(Mod, Type, Encoded, ExpectedValue) ->
+    case Mod:encoding_rule() of
+	ber ->
+	    Indefinite = iolist_to_binary(ber_indefinite(Encoded)),
+	    {ok,ExpectedValue} = Mod:decode(Type, Indefinite);
+	_ ->
+	    ok
+    end.
+
+%% Rewrite all definite lengths for constructed values to an
+%% indefinite length.
+ber_indefinite(Bin0) ->
+    case ber_get_tag(Bin0) of
+	done ->
+	    [];
+	primitive ->
+	    Bin0;
+	{constructed,Tag,Bin1} ->
+	    {Len,Bin2} = ber_get_len(Bin1),
+	    <<Val0:Len/binary,Bin/binary>> = Bin2,
+	    Val = iolist_to_binary(ber_indefinite(Val0)),
+	    [<<Tag/binary,16#80,Val/binary,0,0>>|ber_indefinite(Bin)]
+    end.
+
+ber_get_tag(<<>>) ->
+    done;
+ber_get_tag(<<_:2,0:1,_:5,_/binary>>) ->
+    primitive;
+ber_get_tag(<<_:2,1:1,_:5,_/binary>>=Bin0) ->
+    TagLen = ber_tag_length(Bin0),
+    <<Tag:TagLen/binary,Bin/binary>> = Bin0,
+    {constructed,Tag,Bin}.
+
+ber_tag_length(<<_:3,2#11111:5,T/binary>>) ->
+    ber_tag_length_1(T, 1);
+ber_tag_length(_) ->
+    1.
+
+ber_tag_length_1(<<1:1,_:7,T/binary>>, N) ->
+    ber_tag_length_1(T, N+1);
+ber_tag_length_1(<<0:1,_:7,_/binary>>, N) ->
+    N+1.
+
+ber_get_len(<<0:1,L:7,T/binary>>) ->
+    {L,T};
+ber_get_len(<<1:1,Octets:7,T0/binary>>) ->
+    <<L:Octets/unit:8,T/binary>> = T0,
+    {L,T}.
diff --git a/lib/asn1/test/ber_decode_error.erl b/lib/asn1/test/ber_decode_error.erl
index 8be92292ee..6fd2450c62 100644
--- a/lib/asn1/test/ber_decode_error.erl
+++ b/lib/asn1/test/ber_decode_error.erl
@@ -51,4 +51,18 @@ run([]) ->
     {error,{asn1,{invalid_value,_}}} =
 	(catch 'Constructed':decode('I', <<8,7>>)),
 
+    %% Short indefinite length. Make sure that the decoder doesn't look
+    %% beyond the end of binary when looking for a 0,0 terminator.
+    {error,{asn1,{invalid_length,_}}} =
+	(catch 'Constructed':decode('S', sub(<<8,16#80,0,0>>, 3))),
+    {error,{asn1,{invalid_length,_}}} =
+	(catch 'Constructed':decode('S', sub(<<8,16#80,0,0>>, 2))),
+    {error,{asn1,{invalid_length,_}}} =
+	(catch 'Constructed':decode('S', sub(<<40,16#80,1,1,255,0,0>>, 6))),
+    {error,{asn1,{invalid_length,_}}} =
+	(catch 'Constructed':decode('S', sub(<<40,16#80,1,1,255,0,0>>, 5))),
     ok.
+
+sub(Bin, Bytes) ->
+    <<B:Bytes/binary,_/binary>> = Bin,
+    B.