Dongoing created an issue (kamailio/kamailio#4816)
### Description
Several Kamailio IMS modules read a 4-byte Unsigned32 value out of a Diameter
**answer** AVP with
`get_4bytes(avp->data.s)` (`cdp/diameter.h:64`) after only a NULL check — they
do not check
`avp->data.len >= 4`. `get_4bytes` reads four bytes unconditionally, so an AVP
whose value is
shorter than 4 bytes is read past its declared length.
The value comes off the wire from a Diameter peer (the HSS on Cx, the OCS on
Ro/Gy, the PCRF on
Rx), which controls the AVP's length. In the common case the extra bytes read
are the AVP's
4-byte alignment padding (Diameter pads every AVP value, and CDP sizes the
message buffer with
that padding), so the module computes a **wrong integer value** rather than
faulting — a
robustness/correctness defect. If a peer sends the short AVP unpadded at the
very end of the
message the read goes past the message buffer, but even then it is a 1-3 byte
read that does not
reliably fault, and it is not observable in a running node because the received
message lives in
the shared-memory pool (`receiver.c`: `sp->msg = shm_malloc(...)`, and
`msg->buf.s` points into
it), where such a read is caught by neither AddressSanitizer nor
`DBG_SR_MEMORY`. The point is
simply that these sites read four bytes from a field that may be shorter,
without checking — the
fix is to add the length check.
This is systemic — the same unchecked `get_4bytes(...->data.s)` appears across
five modules and
all four IMS Diameter interfaces:
| Module | Interface / answer | Sites (Unsigned32 AVP) |
|---|---|---|
| `ims_icscf/cxdx_avp.c` | Cx UAA/LIA | `:326` Experimental-Result-Code,
`:400`/`:402` Mandatory/Optional-Capability |
| `ims_auth/cxdx_avp.c` | Cx MAA | `:337` Experimental-Result-Code |
| `ims_registrar_scscf/cxdx_avp.c` | Cx SAA | `:333` Experimental-Result-Code |
| `ims_charging/ccr.c` | Ro/Gy CCA | `:368`/`:372` CC-Request-Type/Number,
`:392` CC-Time, `:404` Validity-Time, `:408` Time-Quota-Threshold, `:411`
Result-Code, `:420` Final-Unit-Action, `:483` |
| `ims_qos/rx_avp.c` | Rx AAA | `:1329`, `:1353` Result-Code, `:1361`
Experimental-Result-Code |
Representative (from `ims_icscf/cxdx_avp.c`, identical shape in the others):
```c
avp = ...FindMatchingAVP(..., AVP_IMS_Experimental_Result_Code, ...);
if(!avp || !avp->data.s) { ... return 0; } /* NULL check only, no length
check */
*data = get_4bytes(avp->data.s); /* reads data.s[0..3] */
```
`get_4bytes(_b)` expands to `_b[0]<<24 | _b[1]<<16 | _b[2]<<8 | _b[3]`.
That this is an oversight is shown by the sibling readers in the same files
that **do** check the
length first:
```c
/* ims_icscf/cxdx_avp.c:556, ims_auth:585, ims_registrar_scscf:581 --
SIP-Item-Number */
if(!avp || avp->data.len != 4)
return 0;
*item_number = get_4bytes(avp->data.s);
```
and by CDP's own getter `AAAGetUnsigned32`
(`cdp_avp/avp_get_base_data_format.c:92`), which
rejects `data.len < 4`. The listed sites bypass both. (CDP's wire parser also
uses `get_4bytes`,
but there each read is bounds-checked against the message buffer first — those
are fine.)
There is no XSD or module option in these paths; the modules parse these answer
AVPs on every
Cx/Ro/Gy/Rx transaction, so it applies in the default configuration.
Expected: an Unsigned32 AVP shorter than 4 bytes should be rejected. Actual:
`get_4bytes` reads
4 bytes regardless of `avp->data.len`.
### Troubleshooting
#### Reproduction
1. Peer a Kamailio IMS node with the relevant Diameter peer (I-CSCF/S-CSCF ↔
HSS on Cx, a
charging-enabled node ↔ OCS on Ro/Gy, or a P-CSCF ↔ PCRF on Rx).
2. Cause the corresponding transaction (register a subscriber → UAR/UAA,
MAR/MAA, SAR/SAA; a
charged session → CCR/CCA; a media session → AAR/AAA).
3. Answer with a message whose `Experimental-Result-Code` / `Result-Code` / a
`Capability` / a
CCA quota unit (`CC-Time`, `Validity-Time`, …) AVP carries fewer than 4
bytes of value. The
module reads 4 bytes, i.e. past the AVP's declared value length.
Without a full setup, the read past the value is shown by compiling the
`get_4bytes` +
check-then-read pattern verbatim under AddressSanitizer and feeding it a 1-byte
AVP
(`repro_icscf_get4bytes.c`, `gcc -fsanitize=address`).
#### Debugging Data
AddressSanitizer on the standalone reproducer (verbatim `get_4bytes` macro +
the check-then-read
from `cxdx_avp.c`), with a 1-byte AVP value:
```
==ERROR: AddressSanitizer: heap-buffer-overflow
READ of size 1 at 0x... thread T0
#0 read_u32_avp repro_icscf_get4bytes.c:39 [== get_4bytes(avp->data.s):
cxdx_avp.c:326 / ccr.c:411 / rx_avp.c:1353 ...]
#1 main repro_icscf_get4bytes.c:57
0x... is located 0 bytes to the right of 1-byte region (the AVP value < 4
bytes)
SUMMARY: AddressSanitizer: heap-buffer-overflow
```
(Full log: `evidence/asan-bug6-icscf-oob-read.log`. The mechanism is identical
at every site
listed above. This isolated harness — where the value is a plain `malloc()`
allocation — is the
evidence for the read; in a running node the received message is in the
shared-memory pool, so
the over-read is not observable by ASAN or `DBG_SR_MEMORY`, as noted in the
Description.)
#### Log Messages
```
(None — the read is silent; it does not fault on a normal allocator.)
```
#### SIP Traffic
Not SIP — the trigger is a Diameter answer (UAA/LIA/MAA/SAA on Cx, CCA on
Ro/Gy, AAA on Rx)
carrying an Unsigned32 AVP with fewer than 4 bytes of value, e.g. an
`Experimental-Result`(297)
grouped AVP whose `Experimental-Result-Code`(298) child holds 3 bytes.
### Possible Solutions
Check the length before every `get_4bytes(avp->data.s)` on wire data (or route
them through
`AAAGetUnsigned32`), mirroring the existing checked readers:
```c
if(!avp || !avp->data.s || avp->data.len < 4)
return 0; /* or skip this AVP */
*data = get_4bytes(avp->data.s);
```
Apply at all sites in the table above (`ims_icscf`, `ims_auth`,
`ims_registrar_scscf`,
`ims_charging`, `ims_qos`).
### Additional Information
- [x] LLM/AI Assistants were involved in discovery, analysis or submission of
the issue
- [ ] It happened in a production deployment
- [x] It happened in a testing or development deployment
- [ ] Testing or fuzzing tools were used to generate SIP traffic when it
happened
* **Kamailio Version** - output of `kamailio -v`
```
version: kamailio 5.5.0 (x86_64/linux) d730fa-dirty
compiled with gcc 11.4.0
```
(Code paths unchanged on current `master`.)
* **Operating System**:
```
Ubuntu 22.04 (jammy), x86_64, Linux — Docker container
[bug1-scscf-crash.log](https://github.com/user-attachments/files/29854305/bug1-scscf-crash.log)
--
Reply to this email directly or view it on GitHub:
https://github.com/kamailio/kamailio/issues/4816
You are receiving this because you are subscribed to this thread.
Message ID: <kamailio/kamailio/issues/[email protected]>_______________________________________________
Kamailio - Development Mailing List -- [email protected]
To unsubscribe send an email to [email protected]
Important: keep the mailing list in the recipients, do not reply only to the
sender!
