pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/43233?usp=email )
Change subject: iuup: Improve validation of header size ......................................................................
iuup: Improve validation of header size
* Validate minimum header length of all IuUP packet types. * Use the existing packed structs to get the expected size of the header * Move IUUP_PDU_T_DATA_NOCRC case below the IUUP_PDU_T_DATA_CRC case, to match spec and numbering order (0,1,14). * The IUUP_PDU_T_DATA_NOCRC length check was wrong, it should have been <3, this is now fixed when checking agains the packed struct. Change-Id: I6976003a8b9761ade978c430613341b9eeb52d9c --- M include/osmocom/gsm/protocol/gsm_25_415.h M src/gsm/iuup.c 2 files changed, 13 insertions(+), 9 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/33/43233/1
diff --git a/include/osmocom/gsm/protocol/gsm_25_415.h b/include/osmocom/gsm/protocol/gsm_25_415.h index 5c4dd2b..f45b87a 100644 --- a/include/osmocom/gsm/protocol/gsm_25_415.h +++ b/include/osmocom/gsm/protocol/gsm_25_415.h @@ -158,9 +158,9 @@
/* 3GPP TS 25.415 Section 6.6.2 + 6.6.3.1 */ enum iuup_pdu_type { - IUUP_PDU_T_DATA_CRC = 0, - IUUP_PDU_T_DATA_NOCRC = 1, - IUUP_PDU_T_CONTROL = 14, + IUUP_PDU_T_DATA_CRC = 0, /* PDU Type 0 */ + IUUP_PDU_T_DATA_NOCRC = 1, /* PDU Type 1 */ + IUUP_PDU_T_CONTROL = 14, /* PDU Type 14 */ };
/* 3GPP TS 25.415 Section 6.6.3.2 */ diff --git a/src/gsm/iuup.c b/src/gsm/iuup.c index df303e5..0952c5e 100644 --- a/src/gsm/iuup.c +++ b/src/gsm/iuup.c @@ -871,7 +871,7 @@ struct iuup_pdutype0_hdr *t0h; struct iuup_pdutype14_hdr *t14h;
- if (len < 3) + if (len < 3) /* common minimum length for all IuUP packet types */ return -EINVAL;
header_crc_computed = osmo_iuup_compute_header_crc(data, len); @@ -881,18 +881,22 @@ return -EIO; } switch (pdu_type) { - case IUUP_PDU_T_DATA_NOCRC: - if (len < 4) + case IUUP_PDU_T_DATA_CRC: /* PDU Type 0 */ + if (len < sizeof(struct iuup_pdutype0_hdr)) return -EINVAL; - break; - case IUUP_PDU_T_DATA_CRC: t0h = (struct iuup_pdutype0_hdr *) data; payload_crc = ((uint16_t)t0h->payload_crc_hi << 8) | t0h->payload_crc_lo; payload_crc_computed = osmo_iuup_compute_payload_crc(data, len); if (payload_crc != payload_crc_computed) goto payload_crc_err; break; - case IUUP_PDU_T_CONTROL: + case IUUP_PDU_T_DATA_NOCRC: /* PDU Type 1 */ + if (len < sizeof(struct iuup_pdutype1_hdr)) + return -EINVAL; + break; + case IUUP_PDU_T_CONTROL: /* PDU Type 14 */ + if (len < sizeof(struct iuup_pdutype14_hdr)) + return -EINVAL; t14h = (struct iuup_pdutype14_hdr *) data; if (t14h->ack_nack == IUUP_AN_PROCEDURE) { payload_crc = ((uint16_t)t14h->payload_crc_hi << 8) | t14h->payload_crc_lo;
-
pespin