sms-part-cdma: fix invalid reads due to wrong byte counts in read_bits

  ==174467== Invalid read of size 1
  ==174467==    at 0x10B80C: read_bits (mm-sms-part-cdma.c:255)
  ==174467==    by 0x10B886: read_bits (mm-sms-part-cdma.c:260)
  ==174467==    by 0x10DC2F: read_bearer_data_user_data (mm-sms-part-cdma.c:882)
  ==174467==    by 0x10DC2F: read_bearer_data (mm-sms-part-cdma.c:1000)
  ==174467==    by 0x10DC2F: mm_sms_part_cdma_new_from_binary_pdu (mm-sms-part-cdma.c:1180)
  ==174467==    by 0x10DF24: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:331)
  ==174467==    by 0x10A91D: common_test_valid_part_from_hexpdu (test-sms-part-cdma.c:114)
  ==174467==    by 0x10B0AC: common_test_valid_part_from_pdu (test-sms-part-cdma.c:126)
  ==174467==    by 0x10B0AC: test_invalid_ascii_user_data (test-sms-part-cdma.c:412)
  ==174467==    by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==  Address 0x51a6457 is 0 bytes after a block of size 7 alloc'd
  ==174467==    at 0x48455EF: calloc (vg_replace_malloc.c:1328)
  ==174467==    by 0x49DF6C0: g_malloc0 (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x48ABD24: mm_utils_hexstr2bin (mm-common-helpers.c:1884)
  ==174467==    by 0x10DF06: mm_sms_part_cdma_new_from_pdu (mm-sms-part-cdma.c:325)
  ==174467==    by 0x10A91D: common_test_valid_part_from_hexpdu (test-sms-part-cdma.c:114)
  ==174467==    by 0x10B0AC: common_test_valid_part_from_pdu (test-sms-part-cdma.c:126)
  ==174467==    by 0x10B0AC: test_invalid_ascii_user_data (test-sms-part-cdma.c:412)
  ==174467==    by 0x4A0264D: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A023B4: ??? (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)
  ==174467==    by 0x4A02B1A: g_test_run_suite (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7400.2)

2 files changed, 33 insertions, 15 deletions

diff --git a/src/mm-sms-part-cdma.c b/src/mm-sms-part-cdma.c
index d02e9dc5..3210d608 100644
--- a/src/mm-sms-part-cdma.c
+++ b/src/mm-sms-part-cdma.c
@@ -789,32 +789,39 @@ read_bearer_data_user_data (MMSmsPart              *sms_part,
         }                           \
     } while (0)
 
-#define SUBPARAMETER_SIZE_CHECK(required_size)                             \
-    if (subparameter->parameter_len < required_size) {                  \
-        mm_obj_dbg (log_object, "        cannot read user data, need at least %u bytes (got %u)", \
-                required_size,                                          \
-                subparameter->parameter_len);                           \
-        return;                                                         \
-    }
+#define SUBPARAMETER_SIZE_CHECK_BITS(required_bits)                     \
+    do {                                                                \
+        guint required_bytes;                                           \
+                                                                        \
+        required_bytes = byte_offset + ((bit_offset + required_bits) / 8); \
+        if ((bit_offset + required_bits) % 8)                           \
+            required_bytes++;                                           \
+        if (subparameter->parameter_len < required_bytes) {             \
+            mm_obj_dbg (log_object, "        cannot read user data, need at least %u bytes (got %u)", \
+                        required_bytes,                                 \
+                        subparameter->parameter_len);                   \
+            return;                                                     \
+        }                                                               \
+    } while (0)
 
     g_assert (subparameter->parameter_id == SUBPARAMETER_ID_USER_DATA);
 
     /* Message encoding */
-    SUBPARAMETER_SIZE_CHECK (1);
+    SUBPARAMETER_SIZE_CHECK_BITS (5);
     message_encoding = read_bits (&subparameter->parameter_value[byte_offset], bit_offset, 5);
     OFFSETS_UPDATE (5);
     mm_obj_dbg (log_object, "            message encoding: %s", encoding_to_string (message_encoding));
 
     /* Message type, only if extended protocol message */
     if (message_encoding == ENCODING_EXTENDED_PROTOCOL_MESSAGE) {
-        SUBPARAMETER_SIZE_CHECK (2);
+        SUBPARAMETER_SIZE_CHECK_BITS (8);
         message_type = read_bits (&subparameter->parameter_value[byte_offset], bit_offset, 8);
         OFFSETS_UPDATE (8);
         mm_obj_dbg (log_object, "            message type: %u", message_type);
     }
 
     /* Number of fields */
-    SUBPARAMETER_SIZE_CHECK (byte_offset + 1 + ((bit_offset + 8) / 8));
+    SUBPARAMETER_SIZE_CHECK_BITS (8);
     num_fields = read_bits (&subparameter->parameter_value[byte_offset], bit_offset, 8);
     OFFSETS_UPDATE (8);
     mm_obj_dbg (log_object, "            num fields: %u", num_fields);
@@ -825,7 +832,7 @@ read_bearer_data_user_data (MMSmsPart              *sms_part,
         GByteArray *data;
         guint i;
 
-        SUBPARAMETER_SIZE_CHECK (byte_offset + 1 + ((bit_offset + (num_fields * 8)) / 8));
+        SUBPARAMETER_SIZE_CHECK_BITS (num_fields * 8);
 
         data = g_byte_array_sized_new (num_fields);
         g_byte_array_set_size (data, num_fields);
@@ -875,7 +882,7 @@ read_bearer_data_user_data (MMSmsPart              *sms_part,
             break;
         }
 
-        SUBPARAMETER_SIZE_CHECK (byte_offset + ((bit_offset + (num_fields * 7)) / 8));
+        SUBPARAMETER_SIZE_CHECK_BITS (num_fields * 7);
 
         text = g_malloc (num_fields + 1);
         for (i = 0; i < num_fields; i++) {
@@ -900,7 +907,7 @@ read_bearer_data_user_data (MMSmsPart              *sms_part,
             break;
         }
 
-        SUBPARAMETER_SIZE_CHECK (byte_offset + 1 + ((bit_offset + (num_fields * 8)) / 8));
+        SUBPARAMETER_SIZE_CHECK_BITS (num_fields * 8);
 
         latin = g_malloc (num_fields + 1);
         for (i = 0; i < num_fields; i++) {
@@ -936,7 +943,7 @@ read_bearer_data_user_data (MMSmsPart              *sms_part,
         /* 2 bytes per field! */
         num_bytes = num_fields * 2;
 
-        SUBPARAMETER_SIZE_CHECK (byte_offset + 1 + ((bit_offset + (num_bytes * 8)) / 8));
+        SUBPARAMETER_SIZE_CHECK_BITS (num_bytes * 8);
 
         utf16 = g_malloc (num_bytes);
         for (i = 0; i < num_bytes; i++) {
@@ -961,7 +968,7 @@ read_bearer_data_user_data (MMSmsPart              *sms_part,
     }
 
 #undef OFFSETS_UPDATE
-#undef SUBPARAMETER_SIZE_CHECK
+#undef SUBPARAMETER_SIZE_CHECK_BITS
 }
 
 static void
diff --git a/src/tests/test-sms-part-cdma.c b/src/tests/test-sms-part-cdma.c
index 61a12e71..26b4d008 100644
--- a/src/tests/test-sms-part-cdma.c
+++ b/src/tests/test-sms-part-cdma.c
@@ -402,6 +402,16 @@ test_empty_ascii_user_data (void)
     common_test_valid_part_from_pdu (pdu, sizeof (pdu));
 }
 
+static void
+test_invalid_ascii_user_data (void)
+{
+    static const guint8 pdu[] = {
+        0x0, 0x8, 0x4, 0x1, 0x2, 0x10, 0xe };
+
+    /* valid but don't care about exact details */
+    common_test_valid_part_from_pdu (pdu, sizeof (pdu));
+}
+
 /********************* PDU CREATOR TESTS *********************/
 
 static void
@@ -609,6 +619,7 @@ int main (int argc, char **argv)
     g_test_add_func ("/MM/SMS/CDMA/PDU-Parser/unicode-encoding", test_unicode_encoding);
     g_test_add_func ("/MM/SMS/CDMA/PDU-Parser/empty-unicode-user-data", test_empty_unicode_user_data);
     g_test_add_func ("/MM/SMS/CDMA/PDU-Parser/empty-ascii-user-data", test_empty_ascii_user_data);
+    g_test_add_func ("/MM/SMS/CDMA/PDU-Parser/invalid-ascii-user-data", test_invalid_ascii_user_data);
 
     g_test_add_func ("/MM/SMS/CDMA/PDU-Creator/ascii-encoding", test_create_pdu_text_ascii_encoding);
     g_test_add_func ("/MM/SMS/CDMA/PDU-Creator/latin-encoding", test_create_pdu_text_latin_encoding);