pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/43416?usp=email )
Change subject: sccp2sua: sccp_to_xua_opt(): Fix read buffer overflow on truncated msg ...................................................................... sccp2sua: sccp_to_xua_opt(): Fix read buffer overflow on truncated msg If a malformed/truncated message is received which lacks contents where the last byte of the (1-2 bytes) length Indicator should be, we'd be reading outside of the msgb data. See how in the switch statement the first offset is one less than the follow-up checks. Clean up the function by increasing the pointer on each state in order to fix it. Related: OS#7075 Reported-By: Tristan Madani <[email protected]> Change-Id: I8f9a20b4eff4f4fd76f6c5a7e87ac0afb5921a8b --- M src/sccp2sua.c 1 file changed, 20 insertions(+), 18 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve pespin: Looks good to me, approved Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/src/sccp2sua.c b/src/sccp2sua.c index 5460732..80c24ab 100644 --- a/src/sccp2sua.c +++ b/src/sccp2sua.c @@ -807,7 +807,7 @@ /* some bounds checking */ if (ptr_opt < msg->data) return NULL; - if (ptr_opt > msg->tail - (ptr_opt_is_long ? 2 : 1)) + if (ptr_opt + (ptr_opt_is_long ? 2 : 1) > msg->tail) return NULL; if (ptr_opt_is_long) @@ -831,34 +831,36 @@ enum sccp_parameter_name_codes opt_type = 0; /* dummy value not used */ while (oneopt < msg->tail) { - uint8_t opt_len; - uint16_t opt_len16; + uint8_t len_size; + uint16_t opt_len; + opt_type = oneopt[0]; + oneopt++; switch (opt_type) { case SCCP_PNC_END_OF_OPTIONAL: return xua; case SCCP_PNC_LONG_DATA: - /* two byte length field */ - if (oneopt + 2 > msg->tail) + /* two byte "Length Indicator" */ + len_size = 2; + if (oneopt + len_size > msg->tail) goto malformed; - opt_len16 = oneopt[1] << 8 | oneopt[2]; - if (oneopt + 3 + opt_len16 > msg->tail) - goto malformed; - xua_msg_add_sccp_opt(xua, opt_type, opt_len16, oneopt+3); - oneopt += 3 + opt_len16; + opt_len = oneopt[0] << 8 | oneopt[1]; + oneopt += len_size; break; default: - /* one byte length field */ - if (oneopt + 1 > msg->tail) + /* one byte "Length Indicator" */ + len_size = 1; + if (oneopt + len_size > msg->tail) goto malformed; - - opt_len = oneopt[1]; - if (oneopt + 2 + opt_len > msg->tail) - goto malformed; - xua_msg_add_sccp_opt(xua, opt_type, opt_len, oneopt+2); - oneopt += 2 + opt_len; + opt_len = oneopt[0]; + oneopt += len_size; } + + if (oneopt + opt_len > msg->tail) + goto malformed; + xua_msg_add_sccp_opt(xua, opt_type, opt_len, oneopt); + oneopt += opt_len; } LOGP(DLSUA, LOGL_ERROR, "Parameter %s not found\n", osmo_sccp_pnc_name(SCCP_PNC_END_OF_OPTIONAL)); return NULL; -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/43416?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: libosmo-sigtran Gerrit-Branch: master Gerrit-Change-Id: I8f9a20b4eff4f4fd76f6c5a7e87ac0afb5921a8b Gerrit-Change-Number: 43416 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: osmith <[email protected]> Gerrit-Reviewer: pespin <[email protected]>
