[M] Change in libosmo-sigtran[master]: sccp: Validate SUA address parsing succeeds [1/3]

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sigtran/+/40289?usp=email ) Change subject: sccp: Validate SUA address parsing succeeds [1/3] ...................................................................... sccp: Validate SUA address parsing succeeds [1/3] Make sure sua_addr_parse() is successful everywhere to avoid checking on potentially uninitialized memory otherwise. This patch indeed uncovers a sua_addr_parse() failure in sccp_scrc_rx_scoc_conn_msg() triggered eg. when submitting a RANAP msg over SCCP in osmo-hnbgw. This happens because Connection Oriented messages (!= CREQ) contain no "Called Address". This is issue is fixed in the 2 patches following up this one. Change-Id: I36387eb53e160425844b20a296fa5dc75db1ec54 --- M src/sccp_sclc.c M src/sccp_scrc.c 2 files changed, 73 insertions(+), 13 deletions(-) Approvals: laforge: Looks good to me, approved Jenkins Builder: Verified daniel: Looks good to me, but someone else must approve diff --git a/src/sccp_sclc.c b/src/sccp_sclc.c index 9d424fa..7fee2a3 100644 --- a/src/sccp_sclc.c +++ b/src/sccp_sclc.c @@ -44,6 +44,7 @@ */ #include <string.h> +#include <errno.h> #include <osmocom/core/utils.h> #include <osmocom/core/linuxlist.h> @@ -188,6 +189,7 @@ struct msgb *upmsg; struct osmo_sccp_user *scu; uint32_t protocol_class; + int rc; if (!data_ie) { LOGPSCI(inst, LOGL_ERROR, "SCCP/SUA CLDT without user data?!?\n"); @@ -201,8 +203,23 @@ osmo_prim_init(&prim->oph, SCCP_SAP_USER, OSMO_SCU_PRIM_N_UNITDATA, PRIM_OP_INDICATION, upmsg); - sua_addr_parse(&param->called_addr, xua, SUA_IEI_DEST_ADDR); - sua_addr_parse(&param->calling_addr, xua, SUA_IEI_SRC_ADDR); + + rc = sua_addr_parse(&param->called_addr, xua, SUA_IEI_DEST_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid DEST_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + msgb_free(upmsg); + return -EINVAL; + } + + rc = sua_addr_parse(&param->calling_addr, xua, SUA_IEI_SRC_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid SRC_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + msgb_free(upmsg); + return -EINVAL; + } + param->in_sequence_control = xua_msg_get_u32(xua, SUA_IEI_SEQ_CTRL); protocol_class = xua_msg_get_u32(xua, SUA_IEI_PROTO_CLASS); param->return_option = protocol_class & 0x80; @@ -210,7 +227,6 @@ scu = sccp_user_find(inst, param->called_addr.ssn, param->called_addr.pc); - if (!scu) { /* FIXME: Send destination unreachable? */ _LOGPSCI(inst, DLSUA, LOGL_NOTICE, "Received SUA message for unequipped SSN %u\n", @@ -237,6 +253,7 @@ struct xua_msg_part *data_ie = xua_msg_find_tag(xua, SUA_IEI_DATA); struct msgb *upmsg; struct osmo_sccp_user *scu; + int rc; if (!data_ie) { LOGPSCI(inst, LOGL_ERROR, "SCCP/SUA CLDR without user data?!?\n"); @@ -251,8 +268,22 @@ OSMO_SCU_PRIM_N_NOTICE, PRIM_OP_INDICATION, upmsg); - sua_addr_parse(&param->called_addr, xua, SUA_IEI_DEST_ADDR); - sua_addr_parse(&param->calling_addr, xua, SUA_IEI_SRC_ADDR); + rc = sua_addr_parse(&param->called_addr, xua, SUA_IEI_DEST_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid DEST_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + msgb_free(upmsg); + return -EINVAL; + } + + rc = sua_addr_parse(&param->calling_addr, xua, SUA_IEI_SRC_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid SRC_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + msgb_free(upmsg); + return -EINVAL; + } + param->importance = xua_msg_get_u32(xua, SUA_IEI_IMPORTANCE); param->cause = xua_msg_get_u32(xua, SUA_IEI_CAUSE); @@ -335,13 +366,14 @@ /* Optional: Data */ xua_msg_copy_part(xua_out, SUA_IEI_DATA, xua_in, SUA_IEI_DATA); - sua_addr_parse(&called, xua_out, SUA_IEI_DEST_ADDR); + OSMO_ASSERT(sua_addr_parse(&called, xua_out, SUA_IEI_DEST_ADDR) == 0); + /* Route on PC + SSN ? */ if (called.ri == OSMO_SCCP_RI_SSN_PC) { /* if no PC, copy OPC into called addr */ if (!(called.presence & OSMO_SCCP_ADDR_T_PC)) { struct osmo_sccp_addr calling; - sua_addr_parse(&calling, xua_out, SUA_IEI_SRC_ADDR); + OSMO_ASSERT(sua_addr_parse(&calling, xua_out, SUA_IEI_SRC_ADDR) == 0); called.presence |= OSMO_SCCP_ADDR_T_PC; called.pc = calling.pc; /* Re-encode / replace called address */ diff --git a/src/sccp_scrc.c b/src/sccp_scrc.c index abf86e4..990987a 100644 --- a/src/sccp_scrc.c +++ b/src/sccp_scrc.c @@ -21,6 +21,7 @@ */ #include <stdbool.h> +#include <errno.h> #include <osmocom/core/logging.h> #include <osmocom/core/msgb.h> @@ -443,10 +444,16 @@ struct xua_msg *xua) { struct osmo_sccp_addr called; + int rc; LOGPSCI(inst, LOGL_DEBUG, "%s: %s\n", __func__, xua_msg_dump(xua, &xua_dialect_sua)); - sua_addr_parse(&called, xua, SUA_IEI_DEST_ADDR); + rc = sua_addr_parse(&called, xua, SUA_IEI_DEST_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid DEST_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + return -EINVAL; + } /* Is this a CR message ? */ if (xua->hdr.msg_type != SUA_CO_CORE) @@ -465,10 +472,16 @@ struct xua_msg *xua) { struct osmo_sccp_addr called; + int rc; LOGPSCI(inst, LOGL_DEBUG, "%s: %s\n", __func__, xua_msg_dump(xua, &xua_dialect_sua)); - sua_addr_parse(&called, xua, SUA_IEI_DEST_ADDR); + rc = sua_addr_parse(&called, xua, SUA_IEI_DEST_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid DEST_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + return -EINVAL; + } /* Message Type */ if (xua->hdr.msg_type == SUA_CL_CLDR) { @@ -487,12 +500,18 @@ } /* ensure the CallingParty address doesn't just contain SSN, but at least SSN+PC */ -static void ensure_opc_in_calling_ssn(struct osmo_sccp_instance *inst, +static int ensure_opc_in_calling_ssn(struct osmo_sccp_instance *inst, struct xua_msg *xua) { struct osmo_sccp_addr calling; + int rc; - sua_addr_parse(&calling, xua, SUA_IEI_SRC_ADDR); + rc = sua_addr_parse(&calling, xua, SUA_IEI_SRC_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid SRC_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + return -EINVAL; + } /* if we route on SSN and only have a SSN in the address... */ if (calling.ri == OSMO_SCCP_RI_SSN_PC && @@ -504,6 +523,7 @@ xua_msg_free_tag(xua, SUA_IEI_SRC_ADDR); xua_msg_add_sccp_addr(xua, SUA_IEI_SRC_ADDR, &calling); } + return 0; } /* Figure C.1/Q.714 Sheet 1 of 12, after we converted the @@ -514,6 +534,7 @@ struct osmo_sccp_addr called; uint32_t proto_class; struct xua_msg_part *hop_ctr_part; + int rc; LOGPSCI(inst, LOGL_DEBUG, "%s: %s\n", __func__, xua_msg_dump(xua, &xua_dialect_sua)); /* TODO: SCCP or nodal congestion? */ @@ -528,9 +549,16 @@ /* We only treat connectionless and CR below */ /* ensure we have at least OPC+SSN and not just SSN in CallingParty (OS#5146) */ - ensure_opc_in_calling_ssn(inst, xua); + rc = ensure_opc_in_calling_ssn(inst, xua); + if (rc < 0) + return -EINVAL; - sua_addr_parse(&called, xua, SUA_IEI_DEST_ADDR); + rc = sua_addr_parse(&called, xua, SUA_IEI_DEST_ADDR); + if (rc < 0) { + LOGPSCI(inst, LOGL_ERROR, "XUA Message %s without valid DEST_ADDR\n", + xua_hdr_dump(xua, &xua_dialect_sua)); + return -EINVAL; + } /* Route on GT? */ if (called.ri != OSMO_SCCP_RI_GT) { -- To view, visit https://gerrit.osmocom.org/c/libosmo-sigtran/+/40289?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: I36387eb53e160425844b20a296fa5dc75db1ec54 Gerrit-Change-Number: 40289 Gerrit-PatchSet: 3 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel &lt;dwillmann(a)sysmocom.de&gt; Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-CC: osmith &lt;osmith(a)sysmocom.de&gt;