sms-part-3gpp: fix invalid memory read due to wrong size check when reading address

Before the actual number digits there is always a Type of Address byte
that we were not considering during the size check.

  [debug] parsing PDU (0)...
  [debug]   no SMSC address given
  [debug]   deliver type PDU detected
  [debug]   address size: 1 digits (1 bytes)

  ==90832== Command: ./build/test/mmsmspdu --pdu=001C011C --verbose
  ==90832==
  ==90832== Invalid read of size 1
  ==90832==    at 0x10AC90: sms_semi_octets_to_bcd_string (mm-sms-part-3gpp.c:71)
  ==90832==    by 0x10AC90: sms_decode_address (mm-sms-part-3gpp.c:157)
  ==90832==    by 0x10B0C5: mm_sms_part_3gpp_new_from_binary_pdu (mm-sms-part-3gpp.c:512)
  ==90832==    by 0x10BF77: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:368)
  ==90832==    by 0x10A44D: main (mmsmspdu.c:242)
  ==90832==  Address 0x5199874 is 0 bytes after a block of size 4 alloc'd
  ==90832==    at 0x48455EF: calloc (vg_replace_malloc.c:1328)
  ==90832==    by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==90832==    by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
  ==90832==    by 0x10BF56: mm_sms_part_3gpp_new_from_pdu (mm-sms-part-3gpp.c:362)
  ==90832==    by 0x10A44D: main (mmsmspdu.c:242)

2 files changed, 18 insertions, 8 deletions

diff --git a/src/mm-sms-part-3gpp.c b/src/mm-sms-part-3gpp.c
index 4244de0cd..5d9d637ac 100644
--- a/src/mm-sms-part-3gpp.c
+++ b/src/mm-sms-part-3gpp.c
@@ -123,7 +123,7 @@ sms_string_to_bcd_semi_octets (guint8 *buf, gsize buflen, const char *string)
 /* len is in semi-octets */
 static gchar *
 sms_decode_address (const guint8  *address,
-                    gint           len,
+                    gint           len_digits,
                     GError       **error)
 {
     guint8 addrtype, addrplan;
@@ -138,23 +138,23 @@ sms_decode_address (const guint8  *address,
         guint8                *unpacked = NULL;
         guint32                unpacked_len;
 
-        unpacked = mm_charset_gsm_unpack (address, (len * 4) / 7, 0, &unpacked_len);
+        unpacked = mm_charset_gsm_unpack (address, (len_digits * 4) / 7, 0, &unpacked_len);
         unpacked_array = g_byte_array_new_take (unpacked, unpacked_len);
         utf8 = mm_modem_charset_bytearray_to_utf8 (unpacked_array, MM_MODEM_CHARSET_GSM, FALSE, error);
     } else if (addrtype == SMS_NUMBER_TYPE_INTL &&
                addrplan == SMS_NUMBER_PLAN_TELEPHONE) {
         /* International telphone number, format as "+1234567890" */
-        utf8 = g_malloc (len + 3); /* '+' + digits + possible trailing 0xf + NUL */
+        utf8 = g_malloc (len_digits + 3); /* '+' + digits + possible trailing 0xf + NUL */
         utf8[0] = '+';
-        sms_semi_octets_to_bcd_string (utf8 + 1, address, (len + 1) / 2);
+        sms_semi_octets_to_bcd_string (utf8 + 1, address, (len_digits + 1) / 2);
     } else {
         /*
          * All non-alphanumeric types and plans are just digits, but
          * don't apply any special formatting if we don't know the
          * format.
          */
-        utf8 = g_malloc (len + 2); /* digits + possible trailing 0xf + NUL */
-        sms_semi_octets_to_bcd_string (utf8, address, (len + 1) / 2);
+        utf8 = g_malloc (len_digits + 2); /* digits + possible trailing 0xf + NUL */
+        sms_semi_octets_to_bcd_string (utf8, address, (len_digits + 1) / 2);
     }
 
     return utf8;
@@ -509,7 +509,8 @@ mm_sms_part_3gpp_new_from_binary_pdu (guint         index,
         mm_sms_part_free (sms_part);
         return NULL;
     }
-    PDU_SIZE_CHECK (offset + tp_addr_size_bytes, "cannot read number");
+    /* +1 due to the Type of Address byte */
+    PDU_SIZE_CHECK (offset + 1 + tp_addr_size_bytes, "cannot read number");
     address = sms_decode_address (&pdu[offset], tp_addr_size_digits, error);
     if (!address) {
         g_prefix_error (error, "Couldn't read address: ");
@@ -518,7 +519,7 @@ mm_sms_part_3gpp_new_from_binary_pdu (guint         index,
     }
     mm_sms_part_take_number (sms_part, g_steal_pointer (&address));
     mm_obj_dbg (log_object, "  number parsed: %s", mm_sms_part_get_number (sms_part));
-    offset += (1 + tp_addr_size_bytes); /* +1 due to the Type of Address byte */
+    offset += (1 + tp_addr_size_bytes);
 
     /* ---------------------------------------------------------------------- */
     /* Get timestamps and indexes for TP-PID, TP-DCS and TP-UDL/TP-UD */
diff --git a/src/tests/test-sms-part-3gpp.c b/src/tests/test-sms-part-3gpp.c
index d6573b13a..9411ad2e3 100644
--- a/src/tests/test-sms-part-3gpp.c
+++ b/src/tests/test-sms-part-3gpp.c
@@ -449,6 +449,14 @@ test_pdu_no_address (void)
     common_test_invalid_pdu (pdu, G_N_ELEMENTS (pdu));
 }
 
+static void
+test_pdu_wrong_address_size (void)
+{
+    static const guint8 pdu[] = { 0x00, 0x1C, 0x01, 0x1C };
+
+    common_test_invalid_pdu (pdu, G_N_ELEMENTS (pdu));
+}
+
 /********************* SMS ADDRESS ENCODER TESTS *********************/
 
 static void
@@ -746,6 +754,7 @@ int main (int argc, char **argv)
     g_test_add_func ("/MM/SMS/3GPP/PDU-Parser/pdu-not-stored", test_pdu_not_stored);
     g_test_add_func ("/MM/SMS/3GPP/PDU-Parser/pdu-insufficient-data", test_pdu_insufficient_data);
     g_test_add_func ("/MM/SMS/3GPP/PDU-Parser/pdu-no-address", test_pdu_no_address);
+    g_test_add_func ("/MM/SMS/3GPP/PDU-Parser/pdu-wrong-address-size", test_pdu_wrong_address_size);
 
     g_test_add_func ("/MM/SMS/3GPP/Address-Encoder/smsc-intl", test_address_encode_smsc_intl);
     g_test_add_func ("/MM/SMS/3GPP/Address-Encoder/smsc-unknown", test_address_encode_smsc_unknown);