sigcomp: Add buffer check to STATE-ACCESS

Two conditions were not checked, state_length == 0 && state_begin != 0
and the boundaries of the state buffer. The former is not a big deal,
but the second issue causes a buffer overrun (detected by ASAN).

The buffer size is supposed to be stored in the state buffer, that was
not the case for the initial two SIP SDP and Presence state buffers.
Fix a typo for presence_buf zero-ing while at it.

Bug: 9601
Change-Id: I41dde83185da60b670cca010ecc7b2a2aaaedeb9
Reviewed-on: https://code.wireshark.org/review/1529
Reviewed-by: Anders Broman <a.broman58@gmail.com>

2 files changed, 28 insertions, 8 deletions

diff --git a/epan/sigcomp-udvm.c b/epan/sigcomp-udvm.c
index bd14dab900..2f5d101764 100644
--- a/epan/sigcomp-udvm.c
+++ b/epan/sigcomp-udvm.c
@@ -110,6 +110,7 @@ const value_string result_code_vals[] = {
 	{14,	"Input bytes requested beyond end of message" },
 	{15,	"Maximum number of UDVM cycles reached" },
 	{16,	"UDVM stack underflow" },
+	{17,	"state_length is 0, but state_begin is non-zero" },
 	{ 255,	"This branch isn't coded yet" },
 	{ 0,    NULL }
 };
diff --git a/epan/sigcomp_state_hdlr.c b/epan/sigcomp_state_hdlr.c
index ce355dce45..cdb8782eae 100644
--- a/epan/sigcomp_state_hdlr.c
+++ b/epan/sigcomp_state_hdlr.c
@@ -634,6 +634,8 @@ sigcomp_init_udvm(void){
 	 * Debug 	g_warning("Sigcomp init: Storing partial state =%s",partial_state_str);
 	 */
 	memset(sip_sdp_buff, 0, 8);
+	sip_sdp_buff[0] = SIP_SDP_STATE_LENGTH >> 8;
+	sip_sdp_buff[1] = SIP_SDP_STATE_LENGTH & 0xff;
 	i = 0;
 	while ( i < SIP_SDP_STATE_LENGTH ){
 		sip_sdp_buff[i+8] = sip_sdp_static_dictionaty_for_sigcomp[i];
@@ -654,7 +656,9 @@ sigcomp_init_udvm(void){
 
 	partial_state_str = bytes_to_ep_str(presence_state_identifier, 6);
 
-	memset(sip_sdp_buff, 0, 8);
+	memset(presence_buff, 0, 8);
+	presence_buff[0] = PRESENCE_STATE_LENGTH >> 8;
+	presence_buff[1] = PRESENCE_STATE_LENGTH & 0xff;
 	i = 0;
 	while ( i < PRESENCE_STATE_LENGTH ){
 		presence_buff[i+8] = presence_static_dictionary_for_sigcomp[i];
@@ -672,6 +676,7 @@ int udvm_state_access(tvbuff_t *tvb, proto_tree *tree,guint8 *buff,guint16 p_id_
 	int			result_code = 0;
 	guint32		n;
 	guint16		k;
+	guint16		buf_size_real;
 	guint16		byte_copy_right;
 	guint16		byte_copy_left;
 	char		partial_state[STATE_BUFFER_SIZE]; /* Size is 6 - 20 */
@@ -730,10 +735,7 @@ int udvm_state_access(tvbuff_t *tvb, proto_tree *tree,guint8 *buff,guint16 p_id_
 	 * If k = byte_copy_right then set n := byte_copy_left, else set n := k
 	 *
 	 */
-	/*
-	if ( ( state_begin + state_length ) > sip_sdp_state_length )
-		return 3;
-		*/
+
 	/*
 	 * buff					= Where "state" will be stored
 	 * p_id_start			= Partial state identifier start pos in the buffer(buff)
@@ -745,6 +747,8 @@ int udvm_state_access(tvbuff_t *tvb, proto_tree *tree,guint8 *buff,guint16 p_id_
 	 * FALSE				= Indicates that state_* is in the stored state
 	 */
 
+	buf_size_real = (state_buff[0] << 8) | state_buff[1];
+
 	/*
 	 * The value of
 	 * state_length MUST be taken from the returned item of state in the
@@ -752,9 +756,8 @@ int udvm_state_access(tvbuff_t *tvb, proto_tree *tree,guint8 *buff,guint16 p_id_
 	 *
 	 * The same is true of state_address, state_instruction.
 	 */
-	if ( *state_length == 0 ){
-		*state_length = state_buff[0] << 8;
-		*state_length = *state_length | state_buff[1];
+	if (*state_length == 0) {
+		*state_length = buf_size_real;
 	}
 	if ( *state_address == 0 ){
 		*state_address = state_buff[2] << 8;
@@ -765,6 +768,22 @@ int udvm_state_access(tvbuff_t *tvb, proto_tree *tree,guint8 *buff,guint16 p_id_
 		*state_instruction = *state_instruction | state_buff[5];
 	}
 
+	/*
+	 * Decompression failure occurs if bytes are copied from beyond the end of
+	 * the state_value.
+	 */
+	if ((state_begin + *state_length) > buf_size_real) {
+		return 3;
+	}
+
+	/*
+	 * Note that decompression failure will always occur if the state_length
+	 * operand is set to 0 but the state_begin operand is non-zero.
+	 */
+	if (*state_length == 0 && state_begin != 0) {
+		return 17;
+	}
+
 	n = state_begin + 8;
 	k = *state_address;