A cell broadcast message using unsupported encodings would trigger an
assert in ModemManager and kill the process. A compromised base station
or modem firmware could craft or pass through these messages.
Reported by Yehia Ezzat and Keith Linneman
Impact: Moderate
CVSS:3.1/AV:A/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H (6.5 Medium)
CWE-617 (Reachable Assertion)
Signed-off-by: Dan Williams <dan@ioncontrol.co>
g_cancellable_disconnect() must not be called from within the
cancellable callback, or it will deadlock.
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Add the ID_MM_MAX_TIMEOUTS udev tag to allow users and system
administrators to configure the maximum number of consecutive
command timeouts before a modem is invalidated and re-probed,
on a per-device basis.
The tag accepts an integer value >= 0, where 0 disables the
timeout monitoring feature entirely. If not set, the existing
default of 10 consecutive timeouts is used.
This is needed for some older modems which become unresponsive
during consecutive data traffic, but come back alive when traffic
is lower. These modems have no further firmware updates available.
This follows the same pattern as other udev-configurable device
properties like ID_MM_MAX_MULTIPLEXED_LINKS.
Signed-off-by: Filip Pokryvka <fpokryvk@redhat.com>
Modem MC7304 has firmware flaw that PCO querry makes it unresponsive (firmware is not upgradable, no new release). There are possibily other older modems affected as well, I have no possibility to check. Add udev rule disabling PCO for affected modem (shares productID with RC7611).
Signed-off-by: Filip Pokryvka <fpokryvk@redhat.com>
The test_creg_match helper would only parse the first match, but actual
code e.g. parse_unsolicited() in src/mm-port-serial-at.c iterates over
all matches:
this updates the test to loop similarily to what is done in real code.
There were two tests assuming only the first match gets matched, so
their result were updated, and a third test was added matching the URC
received from a SIM7672 modem (including unrelated CNSMOD message)
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
When receiving a message from modem containing multiple CREG events that
have different format, the parsing happens in the order CREG regexes are
registered, and not in the order the modem sent the events.
That means that when receiving the following URC:
<CR><LF>+CEREG: 2<CR><LF><CR><LF>+CNSMOD: 8<CR><LF><CR><LF>+CEREG: 1,AAAA,01234567,7<CR><LF>
we'd incorrectly parse the `CEREG: 1` before `CEREG: 2`, and remember
the state as searching (2) instead of home (1), which can cause trouble
connecting because of waiting for state to go home which would never
happen again.
This fixes that problem by merging all the regexes together, so the
parsing should properly process the events in matching order in
parse_unsolicited()
Unfortunately changing the regex like this means that
mm_3gpp_parse_creg_response() match info handling also needs to be
reworked: matches number are increased for each regex, so we can
reliably know which regex was matched based on match count.
This is actually cleaner than the old approach that just guessed if nth
field with "item_is_lac_not_stat()", but requires careful testing.
This was validated checking that tests fail if any of the indices is
changed by iterating over them all and running the test:
grep -n 'n_matches -' src/mm-modem-helpers.c | sed -e 's/:.*//' \
| while read lineno; do
echo $lineno
git show HEAD:src/mm-modem-helpers.c \
| awk -v l=$lineno '{ if (NR == l) {
print $1,$2,$3,$4,$5 - 1,";"
} else {
print
}}' > src/mm-modem-helpers.c
ninja -C build src/tests/test-modem-helpers || break
build/src/tests/test-modem-helpers && break
done
A test case was added to make this work, as the Samsung Wave S8500
sample 'act' doesn't seem to actually be a technology value?
(case 36/42 could actually be merged but were kept separate on purpose
for easier maintenance/testing)
Fixes: #1087
Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
Test the fix in MR !1469 that fixes the issue in #840.
If a multi-part PDU is sent from a device to its own number, on receipt
the DELIVER PDU was incorrectly dropped because when comparing the new
parts to existing ones, the comparison found the original SUBMIT PDU.
Signed-off-by: Dan Williams <dan@ioncontrol.co>
The patch resolves a coredump caused by using 'client' instead of 'self'
in mm_obj_warn() call when failing to register tracking events.
Signed-off-by: shuihan-001 <xing1989093@outlook.com>
On my modem, AT!ICCID? command gives response without a preceding `!`:
```
AT!ICCID?
ICCID: 8970162010103256136
OK
```
Signed-off-by: Semyon Maryasin <simeon@maryasin.name>
device_caps_query_ready() declares caps_custom_data_class_str as
g_autofree without an initializer. On every early error return
(command failure such as a transaction timeout or NotOpened, or a
response parse failure) the variable is never assigned, so the
autofree cleanup runs g_free() on uninitialized stack garbage,
crashing ModemManager with a glibc "invalid free" abort (or SIGSEGV)
whenever the device caps query fails during modem initialization.
Fixes: 19c817cfe9 ("broadband-modem-mbim: consistently handle custom data classes")
Closes: #1094
Signed-off-by: Semyon Maryasin <simeon@maryasin.name>
Update parse_unsolicited() to re-check handler->callback on each
iteration of the match dispatch loop. If a URC handler callback clears
itself during dispatch (e.g. to stop listening), subsequent matches in
the same buffer would attempt to call the NULL pointer, causing a crash.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1074
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Use g_steal_pointer() when passing g_autoptr(GError) to
g_task_return_error() to prevent double-free by the automatic cleanup.
GTask takes ownership of the passed GError. If we use g_autoptr(GError)
locally, it will also call g_error_free() when the function returns,
causing a double-free / heap corruption. Stealing the pointer clears the
local variable and prevents the automatic cleanup.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1073
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Defer the validity change (which triggers modem reprobe/removal) to an
idle callback in rdy_handler().
Calling mm_base_modem_set_valid(FALSE) synchronously from the URC
handler triggers the immediate destruction of the modem object while the
serial port is still iterating the unsolicited message handlers. Deferring
the operation to an idle callback and taking a reference to the modem
ensures the modem object is kept alive until parse_unsolicited() has
returned.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1072
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Disconnect the 'list-configs' indication handler immediately inside the
callback. If the modem sends a duplicate indication before the whole
config load sequence completes, it could overwrite the stored
'get-config-info' signal handler ID, orphaning the old handler and
leading to a Use-After-Free when the GTask is completed and the
orphaned handler eventually fires.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1071
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Use g_signal_connect_object() to connect to child list/object signals
in the 3GPP, Messaging, Voice, and Cell Broadcast interfaces.
Passing the raw D-Bus skeleton pointer as user_data to these signals
can lead to a Use-After-Free if the skeleton is disposed of (e.g. on
hot-unplug) while the parent object or child list survives. By using
g_signal_connect_object(), the signal handlers are automatically
disconnected when the D-Bus skeleton is destroyed.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1068
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Introduce a timezone poll session ID to validate timezone load
results. This ensures that when a pending timezone load completes, we
discard the result if a new poll session has been started (e.g. due to
registration state changes) or if polling has been stopped.
This prevents leaking GSource timeout sources and a subsequent
Use-After-Free if the modem is hot-unplugged while a poll is pending.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1067
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Validate the number parameter in Voice.CreateCall and Call.Deflect
D-Bus methods to ensure they do not contain control characters or
semicolons. This prevents AT command injection when the number is
later spliced into ATD or +CTFR commands.
Introduce a common helper mm_utils_is_valid_dial_number() to validate
dial numbers, and add unit tests for it.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1067
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Validate preferred network operator codes in the SIM D-Bus interface
handler to ensure they are numeric. This prevents injecting AT
command delimiters (like CR) into this field, which is later spliced
into AT commands (+CPOL).
This change also plugs the network list leak, that was a problem on
its own.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1065
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Validate PIN and PUK codes in the SIM D-Bus interface handlers to
ensure they are numeric. This prevents injecting AT command
delimiters (like CR) into these fields, which are later spliced
into AT commands (+CPWD, +CLCK, +CPIN).
Introduce a common helper mm_utils_is_numeric() to check if a string
contains only digits, and add unit tests for it.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1065
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
When sending an unencoded USSD command, the user-supplied string was
spliced directly into the +CUSD AT command. If the string contained
CR or semicolons, it could lead to AT command injection.
Use mm_at_quote_string() to properly quote and escape the USSD
command before sending it.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1064
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
When parsing a SUPL address, if the host is not a valid IPv4 address,
validate it as a FQDN (allowing only alphanumeric characters, dots,
and hyphens). This prevents injecting AT command delimiters (like CR
or semicolon) into the host string, which is later spliced into
modem-specific AT commands (+XLCSSLP).
Introduce a common helper mm_utils_is_valid_fqdn() to validate FQDNs,
and add unit tests for it.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1063
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Alphanumeric SMS addresses decoded from GSM-7 could contain control
characters (like CR/LF), which could lead to AT command injection
if the address is later spliced into an AT+CMGS command during
stored SMS send fallback.
Sanitize the decoded address by removing all control characters.
Introduce a common helper mm_utils_remove_control_characters() to
remove control characters from a string in-place, and add unit
tests for it.
https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1062
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
ME910G1 supports decimal format for 4G bands and has also an empty
tdscdma slot between 4G and 4G extended bands commands/responses.
Change shared code and helpers to deal with these differences.
Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
The assumption that if a modem supports a given access tech, the band
settings for that specific tech should be always given is wrong for
4g, since the 4g bands can be disabled setting the value to 0 in
multi-tech modems like ME310M1.
For example, a 3g + 4g modem could have a setting in which all
the 4g bands are unset and only 3g bands are there: this scenario is
not supported by the current code, but it's legitimate.
Remove this limitation and adapt the code to support bands setting
for multi-tech modems.
Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
Fix LN920 current bands test configuration by setting
modem_has_hex_format_4g_bands to TRUE.
The current wrong setting does not cause the test failure since the modem
supports also extended settings and function telit_get_ext_4g_mm_bands
considers both standard and extended bands as hex without checking
the flag, since decimal format for extended bands is not supported.
Signed-off-by: Daniele Palmas <dnlplm@gmail.com>
The original code for xmm7360_byte_array_read_asn_int() returned
signed integers both from the function and from the returned
Xmm7360RpcMsgArg, but didn't handle sign extension on the
value returned from the function. That would lead to different
values between the function return and the Xmm7360RpcMsgArg
value, which seems wrong.
Make those two correspond and add checks in various places that
shouldn't ever see negative ints.
Signed-off-by: Dan Williams <dan@ioncontrol.co>
That way UNKNOWN is the zero'd value instead of BYTE; it's easier
to test for and less error-prone if not explicitly set to the right
value.
Signed-off-by: Dan Williams <dan@ioncontrol.co>
xmm7360_byte_array_read_asn_int() would keep bit-shifting a gint into
oblivion if the int's size was > 32-bit. It also didn't handle size 0
ints at all, and while size 24 ints would get stuffed into a 32-bit
variable those probably aren't a thing and if they actually are they
should get specifically handled.
Signed-off-by: Dan Williams <dan@ioncontrol.co>
So we can unit-test all the parsing and building code more easily
without having to link to a bunch of serial port stuff.
Signed-off-by: Dan Williams <dan@ioncontrol.co>
If the error matches, then a `+CIMI` command is sent to the device and
processing continues. However, the current error (for the `+CPIN?` command)
is not freed or propagated, fix that.
Fixes: 5c1fcbfe4a ("mtk-legacy: read IMSI if CPIN response is an error")
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
The MTK specific modem object should be included in the 'mtk' shared
utils, so that they're available to other plugins.
Signed-off-by: Aleksander Morgado <aleksandermj@google.com>
Dan Williams
Aleksander Morgado
Filip Pokryvka
Dominique Martinet
Semyon Maryasin
Christian Wendt
shuihan-001
Nikolay Savchenko
Andrey Skvortsov
Daniele Palmas
Lukas Voegl
Barnabás Pőcze
Alexander Couzens
Wenbin
xuhongxing