colin/ModemManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

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

Browse Source 
==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)
This commit is contained in:
Aleksander Morgado
2023-03-30 22:19:54 +00:00
parent 1b60330032
commit 11539a2d82
2 changed files with 33 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

31
src/mm-sms-part-cdma.c
View File

@@ -789,32 +789,39 @@ read_bearer_data_user_data (MMSmsPart *sms_part,
} \ } \
} while (0) } while (0)
#define SUBPARAMETER_SIZE_CHECK(required_size) \ #define SUBPARAMETER_SIZE_CHECK_BITS(required_bits) \
if (subparameter->parameter_len < required_size) { \ 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)", \ mm_obj_dbg (log_object, " cannot read user data, need at least %u bytes (got %u)", \
required_size, \ required_bytes, \
subparameter->parameter_len); \ subparameter->parameter_len); \
return; \ return; \
} } \
} while (0)
g_assert (subparameter->parameter_id == SUBPARAMETER_ID_USER_DATA); g_assert (subparameter->parameter_id == SUBPARAMETER_ID_USER_DATA);
/* Message encoding */ /* Message encoding */
SUBPARAMETER_SIZE_CHECK (1); SUBPARAMETER_SIZE_CHECK_BITS (5);
message_encoding = read_bits (&subparameter->parameter_value[byte_offset], bit_offset, 5); message_encoding = read_bits (&subparameter->parameter_value[byte_offset], bit_offset, 5);
OFFSETS_UPDATE (5); OFFSETS_UPDATE (5);
mm_obj_dbg (log_object, " message encoding: %s", encoding_to_string (message_encoding)); mm_obj_dbg (log_object, " message encoding: %s", encoding_to_string (message_encoding));
/* Message type, only if extended protocol message */ /* Message type, only if extended protocol message */
if (message_encoding == ENCODING_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); message_type = read_bits (&subparameter->parameter_value[byte_offset], bit_offset, 8);
OFFSETS_UPDATE (8); OFFSETS_UPDATE (8);
mm_obj_dbg (log_object, " message type: %u", message_type); mm_obj_dbg (log_object, " message type: %u", message_type);
} }
/* Number of fields */ /* 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); num_fields = read_bits (&subparameter->parameter_value[byte_offset], bit_offset, 8);
OFFSETS_UPDATE (8); OFFSETS_UPDATE (8);
mm_obj_dbg (log_object, " num fields: %u", num_fields); mm_obj_dbg (log_object, " num fields: %u", num_fields);
@@ -825,7 +832,7 @@ read_bearer_data_user_data (MMSmsPart *sms_part,
GByteArray *data; GByteArray *data;
guint i; 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); data = g_byte_array_sized_new (num_fields);
g_byte_array_set_size (data, num_fields); g_byte_array_set_size (data, num_fields);
@@ -875,7 +882,7 @@ read_bearer_data_user_data (MMSmsPart *sms_part,
break; 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); text = g_malloc (num_fields + 1);
for (i = 0; i < num_fields; i++) { for (i = 0; i < num_fields; i++) {
@@ -900,7 +907,7 @@ read_bearer_data_user_data (MMSmsPart *sms_part,
break; 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); latin = g_malloc (num_fields + 1);
for (i = 0; i < num_fields; i++) { for (i = 0; i < num_fields; i++) {
@@ -936,7 +943,7 @@ read_bearer_data_user_data (MMSmsPart *sms_part,
/* 2 bytes per field! */ /* 2 bytes per field! */
num_bytes = num_fields * 2; 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); utf16 = g_malloc (num_bytes);
for (i = 0; i < num_bytes; i++) { for (i = 0; i < num_bytes; i++) {
@@ -961,7 +968,7 @@ read_bearer_data_user_data (MMSmsPart *sms_part,
} }
#undef OFFSETS_UPDATE #undef OFFSETS_UPDATE
#undef SUBPARAMETER_SIZE_CHECK #undef SUBPARAMETER_SIZE_CHECK_BITS
} }
static void static void

11
src/tests/test-sms-part-cdma.c
View File

@@ -402,6 +402,16 @@ test_empty_ascii_user_data (void)
common_test_valid_part_from_pdu (pdu, sizeof (pdu)); 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 *********************/ /********************* PDU CREATOR TESTS *********************/
static void 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/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-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/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/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); g_test_add_func ("/MM/SMS/CDMA/PDU-Creator/latin-encoding", test_create_pdu_text_latin_encoding);