From 838856bde63c0c1089d0a4dab42a532e1420444f Mon Sep 17 00:00:00 2001
From: Anders Svensson <anders@erlang.org>
Date: Fri, 11 Oct 2013 13:12:32 +0200
Subject: Fix handling of 5014, DIAMETER_INVALID_AVP_LENGTH

The error was detected as 5004 (DIAMETER_INVALID_AVP_VALUE) for
stringish Diameter types, in which case an AVP length that pointed past
the end of a message resulted in encode failure of the suggested
Failed-AVP.

Should have been fixed in commit 4ce2d3a6.
---
 lib/diameter/src/base/diameter_codec.erl | 20 +++++++++++---------
 lib/diameter/src/base/diameter_types.erl | 21 ++++++++++++++++-----
 2 files changed, 27 insertions(+), 14 deletions(-)

(limited to 'lib/diameter/src/base')

diff --git a/lib/diameter/src/base/diameter_codec.erl b/lib/diameter/src/base/diameter_codec.erl
index 1d647b8c87..0de4d53973 100644
--- a/lib/diameter/src/base/diameter_codec.erl
+++ b/lib/diameter/src/base/diameter_codec.erl
@@ -477,8 +477,11 @@ split_head(<<Code:32, 1:1, M:1, P:1, _:5, Len:24, V:32, _/bitstring>>) ->
 split_head(<<Code:32, 0:1, M:1, P:1, _:5, Len:24, _/bitstring>>) ->
     {Code, undefined, M, P, Len, 8};
 
-split_head(Bin) ->
-    ?THROW({5014, #diameter_avp{data = Bin}}).
+%% Header is truncated: pack_avp/1 will pad to the minimum header
+%% length.
+split_head(B)
+  when is_bitstring(B) ->
+    ?THROW({5014, #diameter_avp{data = B}}).
 
 %% 3588:
 %%
@@ -523,9 +526,8 @@ split_data(_, _, _) ->
 %% split_data/4
 
 split_data(Bin, HdrLen, Len, Pad) ->
-    <<_:HdrLen/binary, T/bitstring>> = Bin,
-    case T of
-        <<Data:Len/binary, _:Pad/binary, Rest/bitstring>> ->
+    case Bin of
+        <<_:HdrLen/binary, Data:Len/binary, _:Pad/binary, Rest/bitstring>> ->
             {Data, Rest};
         _ ->
             invalid_avp_length()
@@ -573,15 +575,15 @@ pack_avp(#diameter_avp{data = {Dict, Name, Value}} = A) ->
     {Name, Type} = Dict:avp_name(Code, Vid),
     pack_avp(A#diameter_avp{data = {Hdr, {Type, Value}}});
 
-pack_avp(#diameter_avp{code = undefined, data = Bin})
-  when is_binary(Bin) ->
+pack_avp(#diameter_avp{code = undefined, data = B})
+  when is_bitstring(B) ->
     %% Reset the AVP Length of an AVP Header resulting from a 5014
     %% error. The RFC doesn't explicitly say to do this but the
     %% receiver can't correctly extract this and following AVP's
     %% without a correct length. On the downside, the header doesn't
     %% reveal if the received header has been padded.
-    Pad = 8*header_length(Bin) - bit_size(Bin),
-    Len = size(<<H:5/binary, _:24, T/binary>> = <<Bin/bitstring, 0:Pad>>),
+    Pad = 8*header_length(B) - bit_size(B),
+    Len = size(<<H:5/binary, _:24, T/binary>> = <<B/bitstring, 0:Pad>>),
     <<H/binary, Len:24, T/binary>>;
 
 %% ... or as an iolist.
diff --git a/lib/diameter/src/base/diameter_types.erl b/lib/diameter/src/base/diameter_types.erl
index 8c07e84777..ca3338be5f 100644
--- a/lib/diameter/src/base/diameter_types.erl
+++ b/lib/diameter/src/base/diameter_types.erl
@@ -92,6 +92,9 @@
   when is_binary(Bin) ->
     binary_to_list(Bin);
 
+'OctetString'(decode, B) ->
+    ?INVALID_LENGTH(B);
+
 'OctetString'(encode = M, zero) ->
     'OctetString'(M, []);
 
@@ -255,9 +258,7 @@
        2 == A, 16 == size(B) ->
     list_to_tuple([N || <<N:A/unit:8>> <= B]);
 
-'Address'(decode, <<A:16, _/binary>> = B)
-  when 1 == A;
-       2 == A ->
+'Address'(decode, B) ->
     ?INVALID_LENGTH(B);
 
 'Address'(encode, T) ->
@@ -278,7 +279,10 @@
     <<_,_/binary>> = 'OctetString'(M, X);
 
 'DiameterIdentity'(decode = M, <<_,_/binary>> = X) ->
-    'OctetString'(M, X).
+    'OctetString'(M, X);
+
+'DiameterIdentity'(decode, X) ->
+    ?INVALID_LENGTH(X).
 
 %% --------------------
 
@@ -286,6 +290,9 @@
   when is_binary(Bin) ->
     scan_uri(Bin);
 
+'DiameterURI'(decode, B) ->
+    ?INVALID_LENGTH(B);
+
 %% The minimal DiameterURI is "aaa://x", 7 characters.
 'DiameterURI'(encode = M, zero) ->
     'OctetString'(M, lists:duplicate(0,7));
@@ -330,9 +337,13 @@
 
 %% --------------------
 
-'UTF8String'(decode, Bin) ->
+'UTF8String'(decode, Bin)
+  when is_binary(Bin) ->
     tl([0|_] = unicode:characters_to_list([0, Bin])); %% assert list return
 
+'UTF8String'(decode, B) ->
+    ?INVALID_LENGTH(B);
+
 'UTF8String'(encode = M, zero) ->
     'UTF8String'(M, []);
 
-- 
cgit v1.2.3