msuraev has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/29083 ) Change subject: SCCP: enforce optional data length limits ...................................................................... SCCP: enforce optional data length limits Limit length of optional Data parameter to 130 bytes to conform with ITU-T Rec Q.713 §4.2..§4.5 while receiving SCCP messages. Related: OS#5579 Change-Id: Icc3bd0a71b29cf61a259c5d97e7dd85beb4397bd --- M include/osmocom/sccp/sccp_types.h M src/sccp.c 2 files changed, 29 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, but someone else must approve msuraev: Looks good to me, approved diff --git a/include/osmocom/sccp/sccp_types.h b/include/osmocom/sccp/sccp_types.h index 0ef52b4..94b8f0b 100644 --- a/include/osmocom/sccp/sccp_types.h +++ b/include/osmocom/sccp/sccp_types.h @@ -24,6 +24,8 @@ #include <osmocom/core/endian.h> #include <osmocom/core/utils.h> +#define SCCP_MAX_OPTIONAL_DATA 130 + /* Table 1/Q.713 - SCCP message types */ enum sccp_message_types { SCCP_MSG_TYPE_CR = 1, diff --git a/src/sccp.c b/src/sccp.c index ee90579..94eca29 100644 --- a/src/sccp.c +++ b/src/sccp.c @@ -20,6 +20,7 @@ * */ +#include <errno.h> #include <string.h> #include <osmocom/core/msgb.h> @@ -27,7 +28,7 @@ #include <osmocom/core/logging.h> #include <osmocom/core/endian.h> #include <osmocom/gsm/tlv.h> - +#include <osmocom/sccp/sccp_types.h> #include <osmocom/sccp/sccp.h> // Unassigned debug area @@ -226,6 +227,12 @@ } if (optional_data.data_len != 0) { + if (optional_data.data_len > SCCP_MAX_OPTIONAL_DATA) { + LOGP(DSCCP, LOGL_ERROR, + "optional data has length %u exceeding max of %u according to ITU-T Rec. Q.713 §4.2\n", + optional_data.data_len, SCCP_MAX_OPTIONAL_DATA); + return -EMSGSIZE; + } msgb->l3h = &msgb->l2h[optional_data.data_start]; result->data_len = optional_data.data_len; } else { @@ -260,6 +267,12 @@ result->destination_local_reference = &rls->destination_local_reference; if (optional_data.data_len != 0) { + if (optional_data.data_len > SCCP_MAX_OPTIONAL_DATA) { + LOGP(DSCCP, LOGL_ERROR, + "optional data has length %u exceeding max of %u according to ITU-T Rec. Q.713 §4.5\n", + optional_data.data_len, SCCP_MAX_OPTIONAL_DATA); + return -EMSGSIZE; + } msgb->l3h = &msgb->l2h[optional_data.data_start]; result->data_len = optional_data.data_len; } else { @@ -297,6 +310,12 @@ /* optional data */ if (optional_data.data_len != 0) { + if (optional_data.data_len > SCCP_MAX_OPTIONAL_DATA) { + LOGP(DSCCP, LOGL_ERROR, + "optional data has length %u exceeding max of %u according to ITU-T Rec. Q.713 §4.4\n", + optional_data.data_len, SCCP_MAX_OPTIONAL_DATA); + return -EMSGSIZE; + } msgb->l3h = &msgb->l2h[optional_data.data_start]; result->data_len = optional_data.data_len; } else { @@ -334,6 +353,12 @@ } if (optional_data.data_len != 0) { + if (optional_data.data_len > SCCP_MAX_OPTIONAL_DATA) { + LOGP(DSCCP, LOGL_ERROR, + "optional data has length %u exceeding max of %u according to ITU-T Rec. Q.713 §4.3\n", + optional_data.data_len, SCCP_MAX_OPTIONAL_DATA); + return -EMSGSIZE; + } msgb->l3h = &msgb->l2h[optional_data.data_start]; result->data_len = optional_data.data_len; } else { @@ -818,7 +843,7 @@ uint8_t extra_size = 3 + 1; int called_len; - if (l3_data && (l3_length < 3 || l3_length > 130)) { + if (l3_data && (l3_length < 3 || l3_length > SCCP_MAX_OPTIONAL_DATA)) { LOGP(DSCCP, LOGL_ERROR, "Invalid amount of data... %zu\n", l3_length); return NULL; } -- To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/29083 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-sccp Gerrit-Branch: master Gerrit-Change-Id: Icc3bd0a71b29cf61a259c5d97e7dd85beb4397bd Gerrit-Change-Number: 29083 Gerrit-PatchSet: 5 Gerrit-Owner: msuraev <msuraev@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy@sysmocom.de> Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: msuraev <msuraev@sysmocom.de> Gerrit-CC: neels <nhofmeyr@sysmocom.de> Gerrit-MessageType: merged