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

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

Browse Source 
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)
This commit is contained in:
Aleksander Morgado
2023-03-30 19:51:04 +00:00
parent bc2aeeb7bd
commit 60ef408cd7
2 changed files with 18 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
src/mm-sms-part-3gpp.c
View File

@@ -123,7 +123,7 @@ sms_string_to_bcd_semi_octets (guint8 *buf, gsize buflen, const char *string)
/* len is in semi-octets */ /* len is in semi-octets */
static gchar * static gchar *
sms_decode_address (const guint8 *address, sms_decode_address (const guint8 *address,
gint len, gint len_digits,
GError **error) GError **error)
{ {
guint8 addrtype, addrplan; guint8 addrtype, addrplan;
@@ -138,23 +138,23 @@ sms_decode_address (const guint8 *address,
guint8 *unpacked = NULL; guint8 *unpacked = NULL;
guint32 unpacked_len; 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); 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); utf8 = mm_modem_charset_bytearray_to_utf8 (unpacked_array, MM_MODEM_CHARSET_GSM, FALSE, error);
} else if (addrtype == SMS_NUMBER_TYPE_INTL && } else if (addrtype == SMS_NUMBER_TYPE_INTL &&
addrplan == SMS_NUMBER_PLAN_TELEPHONE) { addrplan == SMS_NUMBER_PLAN_TELEPHONE) {
/* International telphone number, format as "+1234567890" */ /* 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] = '+'; 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 { } else {
/* /*
* All non-alphanumeric types and plans are just digits, but * All non-alphanumeric types and plans are just digits, but
* don't apply any special formatting if we don't know the * don't apply any special formatting if we don't know the
* format. * format.
*/ */
utf8 = g_malloc (len + 2); /* digits + possible trailing 0xf + NUL */ utf8 = g_malloc (len_digits + 2); /* digits + possible trailing 0xf + NUL */
sms_semi_octets_to_bcd_string (utf8, address, (len + 1) / 2); sms_semi_octets_to_bcd_string (utf8, address, (len_digits + 1) / 2);
} }
return utf8; return utf8;
@@ -509,7 +509,8 @@ mm_sms_part_3gpp_new_from_binary_pdu (guint index,
mm_sms_part_free (sms_part); mm_sms_part_free (sms_part);
return NULL; 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); address = sms_decode_address (&pdu[offset], tp_addr_size_digits, error);
if (!address) { if (!address) {
g_prefix_error (error, "Couldn't read 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_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)); 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 */ /* Get timestamps and indexes for TP-PID, TP-DCS and TP-UDL/TP-UD */

9
src/tests/test-sms-part-3gpp.c
View File

@@ -449,6 +449,14 @@ test_pdu_no_address (void)
common_test_invalid_pdu (pdu, G_N_ELEMENTS (pdu)); 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 *********************/ /********************* SMS ADDRESS ENCODER TESTS *********************/
static void 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-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-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-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-intl", test_address_encode_smsc_intl);
g_test_add_func ("/MM/SMS/3GPP/Address-Encoder/smsc-unknown", test_address_encode_smsc_unknown); g_test_add_func ("/MM/SMS/3GPP/Address-Encoder/smsc-unknown", test_address_encode_smsc_unknown);