ModemManager is the daemon that talks to your cellular modem on most Linux desktops and a lot of embedded gear. It runs as root, it sits on the system D-Bus, and it parses a stream of things the network sends it without asking. One of those things is cell broadcast, the channel that carries emergency alerts and area messages. This is a write up of a single byte in a cell broadcast message that walks the parser into g_assert_not_reached() and takes the whole daemon down with it.
The bug is not a memory corruption and it is not remote code execution. It is an assertion that the author put in as a “this can never happen” guard, sitting on a code path that the network can reach on purpose. That is the shape of it, and it is a common mistake.
At a glance
| Field | Value |
|---|---|
| Class | CWE-617, reachable assertion |
| Component | src/mm-cbm-part.c, mm_cbm_part_new_from_binary_pdu() |
| Impact | Remote DoS of the root daemon, no auth, no interaction |
| CVSS 3.1 | 6.5 (Medium) |
| Vector | AV:A/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:H |
| Trigger | A cell broadcast PDU whose DCS selects an unhandled charset |
| Reach | AT +CBM URC and the QMI broadcast path, both on unsolicited input |
| Affected | ModemManager main at commit 306f6256 |
| Fix | Reject unhandled encodings with a GError instead of asserting |
Cell broadcast, briefly
Cell broadcast is a one-to-many channel. The network pushes a short message to every phone in an area, and it is what carries wireless emergency alerts, the AMBER-alert style messages, and operator area notices. Your modem receives these without any request from the host, hands them up, and ModemManager parses them into its CellBroadcast D-Bus interface. That interface comes up automatically when the modem reports support for it, so there is no opt-in and no configuration a user has to get wrong first.
Two code paths feed the same parser:
- The AT path: a
+CBMunsolicited result code arrives,cbc_cbm_received()insrc/mm-broadband-modem.chandles it, and it calls intomm_cbm_part_new_from_pdu(). - The QMI path:
add_new_read_cbm_part()insrc/mm-broadband-modem-qmi.chandles aGSM_WCDMA_BROADCASTmessage and callsmm_cbm_part_new_from_binary_pdu()directly.
Both end up in the same function, and that function is where the problem is.
The default nobody set
mm_cbm_part_new_from_binary_pdu() starts by allocating the part:
cbm_part = mm_cbm_part_new ();which is a g_slice_new0, so every field is zeroed. The encoding field is an MMSmsEncoding, and in that enum zero is not a neutral placeholder:
typedef enum {
MM_SMS_ENCODING_UNKNOWN = 0x0,
MM_SMS_ENCODING_GSM7,
MM_SMS_ENCODING_8BIT,
MM_SMS_ENCODING_UCS2,
} MMSmsEncoding;So before the parser reads anything, cbm_part->encoding is MM_SMS_ENCODING_UNKNOWN. That is fine as long as every path through the parser overwrites it. Most of them do. One does not.
The data coding scheme, and the two charsets nobody handled
The parser reads the data coding scheme byte and masks off the top four bits to get the coding group, then decides the encoding. The general coding groups and the UDH group share a branch:
} else if ((group == CBS_DATA_CODING_GENERAL_CLASS) ||
(group == CBS_DATA_CODING_GENERAL_NO_CLASS) ||
(group == CBS_DATA_CODING_UDH)) {
guint16 charset = pdu[offset] & CBS_DATA_CODING_GENERAL_CHARSET_MASK;
/* We don't handle compression or 8 bit data */
if (charset == CBS_DATA_CODING_GENERAL_GSM7)
cbm_part->encoding = MM_SMS_ENCODING_GSM7;
else if (charset == CBS_DATA_CODING_GENERAL_UCS2)
cbm_part->encoding = MM_SMS_ENCODING_UCS2;
} else {
g_set_error (error, ...); /* unrecognised group */
return NULL;
}The charset mask covers two bits, so there are four possible charset values: GSM7 (0x00), 8-bit (0x04), UCS2 (0x08), and reserved (0x0C). The branch sets encoding for two of them. The comment even says the quiet part out loud, “We don’t handle compression or 8 bit data,” but not handling it here means the if/else if simply falls off the end. When the charset is 8-bit or reserved, encoding is never assigned, and it keeps the UNKNOWN value that g_slice_new0 gave it. There is no else on this inner branch to catch that.
Take DCS 0x44. The top bits 0b0100 are the general coding group with no message class. The charset bits 0b01 are 8-bit data. That is a legal DCS per 3GPP TS 23.038 section 5, not a malformed byte. It walks straight through this branch and leaves encoding at UNKNOWN.
The assert
A bit further down, the parser switches on the encoding to work out the message length:
switch (cbm_part->encoding) {
case MM_SMS_ENCODING_GSM7:
len = ((pdu_len - offset) * 8) / 7;
break;
case MM_SMS_ENCODING_UCS2:
len = pdu_len - offset;
if (len % 2)
len--;
break;
case MM_SMS_ENCODING_8BIT:
case MM_SMS_ENCODING_UNKNOWN:
default:
g_assert_not_reached ();
}There it is. UNKNOWN and 8BIT share the default case, and the default case is g_assert_not_reached(). On a value the network chose, the daemon aborts:
ERROR:../src/mm-cbm-part.c:286:mm_cbm_part_new_from_binary_pdu: code should not be reached
Bail out! ERROR:../src/mm-cbm-part.c:286:mm_cbm_part_new_from_binary_pdu: code should not be reachedProcess exits with 134, which is 128 plus SIGABRT.
Every crashing DCS
Rather than reason about which values reach the assert, I ran all 256 of them through the real parser. Because the abort kills the process, each value ran in its own fork(), and the parent recorded whether the child exited normally, returned a clean GError, or died on a signal.
parsed OK : 73
clean GError : 159
abort (SIGABRT) : 24The 24 DCS bytes that reach the assert:
0x44 0x45 0x46 0x47 0x4C 0x4D 0x4E 0x4F
0x54 0x55 0x56 0x57 0x5C 0x5D 0x5E 0x5F
0x94 0x95 0x96 0x97 0x9C 0x9D 0x9E 0x9FThey are exactly the general-class, general-no-class, and UDH groups (0x40, 0x50, 0x90) crossed with the 8-bit and reserved charset bits (0x04, 0x0C). Everything lines up with the source.
The fix
The change is small, which is usual for this class of bug. Instead of only catching an unrecognised coding group, reject any encoding that came out of the whole block still UNKNOWN:
cbm_part->encoding = MM_SMS_ENCODING_GSM7;
else if (charset == CBS_DATA_CODING_GENERAL_UCS2)
cbm_part->encoding = MM_SMS_ENCODING_UCS2;
- } else {
+ }
+
+ if (cbm_part->encoding == MM_SMS_ENCODING_UNKNOWN) {
g_set_error (error,
MM_CORE_ERROR,
MM_CORE_ERROR_FAILED,
"Unhandled cbm message encoding: 0x%02x",
pdu[offset]);
return NULL;
}The old else only ran when the group itself was unrecognised. Turning it into an unconditional post-check catches both that case and the new one, the general or UDH group with an 8-bit or reserved charset, because both leave the field at UNKNOWN. By the time control reaches the encoding switch, encoding is guaranteed to be GSM7 or UCS2, so the g_assert_not_reached() becomes a real internal invariant instead of something the network can trip. The message is dropped rather than decoded, which matches the parser’s own comment about not handling 8-bit data.
What to take from it
The bug is not the missing else. That is where it surfaced. The bug is a g_slice_new0 default that happens to be a real enum value with meaning, combined with a parser that treats “I set the encoding” and “I did not touch the encoding” as the same state until an assert forces the question. A zero-initialised default is only safe if every branch either overwrites it or is prepared to see it. Here one branch did neither, and the place that finally noticed was an assert that assumed it could never be wrong.
And g_assert_not_reached() on attacker-influenced input is a crash you wrote for yourself. It reads like defensive programming, but on a boundary that parses network data it is an abort with extra steps. A parser running as root should turn input it does not like into an error and move on, which is what the fix does.
If you maintain a decoder like this, the audit is worth a minute: does every default: in a decode switch return an error, or does one of them abort on input you do not control?
For the longer story, including why I trust a fork-per-byte sweep over reading the code, see the blog post.