pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/43253?usp=email )
Change subject: smpp: Fix potential write buffer overflow on sms->user_data ...................................................................... smpp: Fix potential write buffer overflow on sms->user_data The writes to sms->user_data in submit_to_sms() were not being validated against the maximum size of the buffer, which could lead into writing past the buffer limits. In order to assure safe encdoing of septets into the buffer, the new libosmocore gsm_septet_pack2() is required. Related: OS#7059 Reported-By: Adam Bedard <[email protected]> Depends: libosmocore.git Change-Id I4baa19007c65275ead3d1fc92462bfb8ab68e036 Change-Id: Iee701a3b033d78244fc10dd8563a094b8de385c0 --- M TODO-RELEASE M src/libsmpputil/smpp_msc.c 2 files changed, 25 insertions(+), 5 deletions(-) Approvals: osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/TODO-RELEASE b/TODO-RELEASE index c8858d8..84a2985 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -9,3 +9,4 @@ #library what description / commit summary line libosmovty >=1.12.1 working optional-multi-choice libosmocore >1.12.0 log_get_context(), log_{get,set}_filter(_data)() +libosmogsm >1.14.1 gsm_septet_pack2() diff --git a/src/libsmpputil/smpp_msc.c b/src/libsmpputil/smpp_msc.c index d68dcc0..5fdba36 100644 --- a/src/libsmpputil/smpp_msc.c +++ b/src/libsmpputil/smpp_msc.c @@ -132,6 +132,7 @@ struct tlv_t *t; int mode; int can_store_sms = ((submit->esm_class & SMPP34_MSG_MODE_MASK) != 2); /* != forward mode */ + int rc; dest = subscr_by_dst(net, submit->dest_addr_npi, submit->dest_addr_ton, @@ -247,21 +248,39 @@ ud_len = *sms_msg + 1; if (ud_len > sms_msg_len) { sms_free(sms); - LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u > sms_msg_len=%u\n", ud_len, - sms_msg_len); + LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u > sms_msg_len=%u\n", + ud_len, sms_msg_len); + return ESME_RINVPARLEN; + } + if (ud_len > sizeof(sms->user_data)) { + sms_free(sms); + LOGP(DLSMS, LOGL_ERROR, "invalid sms_msg_len=%u > %zu\n", + sms_msg_len, sizeof(sms->user_data)); return ESME_RINVPARLEN; } printf("copying %u bytes user data...\n", ud_len); - memcpy(sms->user_data, sms_msg, - OSMO_MIN(ud_len, sizeof(sms->user_data))); + memcpy(sms->user_data, sms_msg, ud_len); sms_msg += ud_len; sms_msg_len -= ud_len; padbits = 7 - (ud_len % 7); } - gsm_septet_pack(sms->user_data+ud_len, sms_msg, sms_msg_len, padbits); + rc = gsm_septet_pack2(sms->user_data + ud_len, sizeof(sms->user_data) - ud_len, + sms_msg, sms_msg_len, padbits); + if (rc < 0) { + sms_free(sms); + LOGP(DLSMS, LOGL_ERROR, "invalid ud_len=%u + sms_msg_len=%u > %zu\n", + ud_len, sms_msg_len, sizeof(sms->user_data)); + return ESME_RINVPARLEN; + } sms->user_data_len = (ud_len*8 + padbits)/7 + sms_msg_len;/* SEPTETS */ /* FIXME: sms->text */ } else { + if (sms_msg_len > sizeof(sms->user_data)) { + sms_free(sms); + LOGP(DLSMS, LOGL_ERROR, "invalid sms_msg_len=%u > %zu\n", + sms_msg_len, sizeof(sms->user_data)); + return ESME_RINVPARLEN; + } memcpy(sms->user_data, sms_msg, sms_msg_len); sms->user_data_len = sms_msg_len; } -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/43253?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Iee701a3b033d78244fc10dd8563a094b8de385c0 Gerrit-Change-Number: 43253 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: osmith <[email protected]> Gerrit-Reviewer: pespin <[email protected]>
