pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/43260?usp=email )

Change subject: ranap: Avoid write buffer overflow on GTPv1U outter IPv6 addr ......................................................................

ranap: Avoid write buffer overflow on GTPv1U outter IPv6 addr

If an IPv6 address was passed by libgtp in pdp->lib->gsnru, sgsn_pdp_ctx_iu_rab_activate() would write 16 bytes to an uint32_t variable, overflowing it and writing further on the stack.

Fix it by simply using the new osmo-iuh API ranap_new_msg_rab_assign_data2() which expects an osmo_sockaddr instead of a uint32_t (which then supports IPv6 too). Using the new APIs to convert the GSNA to an osmo_sockaddr and passing it to osmo-iuh new API then we make sure there's no buffer overflow anymore.

Depends: osmo-iuh.git I3a8800afd03b94349c8acec778ab7003819e80af Related: OS#7060 Reported-By: Adam Bedard adam.bedard@gmail.com Change-Id: I3beb8afcabad8d74b12a6aad845b455ed95761d4 --- M include/osmocom/sgsn/gprs_ranap.h M include/osmocom/sgsn/gtp.h M src/sgsn/gprs_ranap.c M src/sgsn/pdpctx.c 4 files changed, 29 insertions(+), 11 deletions(-)

Approvals: fixeria: Looks good to me, approved osmith: Looks good to me, but someone else must approve Jenkins Builder: Verified

diff --git a/include/osmocom/sgsn/gprs_ranap.h b/include/osmocom/sgsn/gprs_ranap.h index b959b57..9185b39 100644 --- a/include/osmocom/sgsn/gprs_ranap.h +++ b/include/osmocom/sgsn/gprs_ranap.h @@ -3,6 +3,7 @@ #include "config.h"

#include <osmocom/core/msgb.h> +#include <osmocom/core/socket.h>

#ifdef BUILD_IU #include <osmocom/ranap/ranap_ies_defs.h> @@ -26,7 +27,9 @@

int sgsn_ranap_iu_tx(struct msgb *msg, uint8_t sapi); int sgsn_ranap_iu_tx_rab_ps_ass_req(struct ranap_ue_conn_ctx *ue_ctx, - uint8_t rab_id, uint32_t gtp_ip, uint32_t gtp_tei); + uint8_t rab_id, + const struct osmo_sockaddr *gtp_addr, + uint32_t gtp_tei); int sgsn_ranap_iu_tx_sec_mode_cmd(struct ranap_ue_conn_ctx *uectx, struct osmo_auth_vector *vec, int send_ck, int new_key); int sgsn_ranap_iu_tx_common_id(struct ranap_ue_conn_ctx *ue_ctx, const char *imsi); diff --git a/include/osmocom/sgsn/gtp.h b/include/osmocom/sgsn/gtp.h index 0f77a18..aba4f0a 100644 --- a/include/osmocom/sgsn/gtp.h +++ b/include/osmocom/sgsn/gtp.h @@ -3,9 +3,12 @@ #include <stddef.h> #include <stdint.h>

+#include <osmocom/core/socket.h> #include <osmocom/gsm/tlv.h> #include <osmocom/gprs/gprs_bssgp_rim.h>

+#include <osmocom/gtp/pdp.h> + struct gprs_ra_id; struct sgsn_instance; struct sgsn_ggsn_ctx; @@ -27,3 +30,8 @@ struct msgb *msg, uint32_t npdu_len, uint8_t *npdu); int sgsn_delete_pdp_ctx(struct sgsn_pdp_ctx *pctx); int send_act_pdp_cont_acc(struct sgsn_pdp_ctx *pctx); + +static inline int gsna_to_osa(struct osmo_sockaddr *dst, const struct ul16_t *in) +{ + return osmo_sockaddr_from_octets(dst, &in->v[0], in->l); +} diff --git a/src/sgsn/gprs_ranap.c b/src/sgsn/gprs_ranap.c index 1cdb72b..4445235 100644 --- a/src/sgsn/gprs_ranap.c +++ b/src/sgsn/gprs_ranap.c @@ -252,16 +252,19 @@ }

int sgsn_ranap_iu_tx_rab_ps_ass_req(struct ranap_ue_conn_ctx *ue_ctx, - uint8_t rab_id, uint32_t gtp_ip, uint32_t gtp_tei) + uint8_t rab_id, + const struct osmo_sockaddr *gtp_addr, + uint32_t gtp_tei) { struct msgb *msg; bool use_x213_nsap = (ue_ctx->rab_assign_addr_enc == RANAP_NSAP_ADDR_ENC_X213); + char ip_str[INET6_ADDRSTRLEN];

LOGP(DRANAP, LOGL_DEBUG, - "Assigning RAB: rab_id=%u, ggsn_ip=%x, teid_gn=%x, use_x213_nsap=%d\n", - rab_id, gtp_ip, gtp_tei, use_x213_nsap); + "Assigning RAB: rab_id=%u, ggsn_ip=%s, teid_gn=%x, use_x213_nsap=%d\n", + rab_id, osmo_sockaddr_ntop(&gtp_addr->u.sa, ip_str), gtp_tei, use_x213_nsap);

- msg = ranap_new_msg_rab_assign_data(rab_id, gtp_ip, gtp_tei, use_x213_nsap); + msg = ranap_new_msg_rab_assign_data2(rab_id, gtp_addr, gtp_tei, use_x213_nsap); return sgsn_scu_iups_tx_data_req(ue_ctx->rnc->scu_iups, ue_ctx->conn_id, msg); }

diff --git a/src/sgsn/pdpctx.c b/src/sgsn/pdpctx.c index 1fbddab..da825e6 100644 --- a/src/sgsn/pdpctx.c +++ b/src/sgsn/pdpctx.c @@ -173,19 +173,23 @@ { struct sgsn_mm_ctx *mm = pdp->mm; struct ranap_ue_conn_ctx *ue_ctx; - uint32_t ggsn_ip; + struct osmo_sockaddr gtp_addr; + char ip_str[INET6_ADDRSTRLEN];

OSMO_ASSERT(mm->ran_type == MM_CTX_T_UTRAN_Iu); ue_ctx = mm->iu.ue_ctx;

/* Get the IP address for ggsn user plane */ - memcpy(&ggsn_ip, pdp->lib->gsnru.v, pdp->lib->gsnru.l); - ggsn_ip = htonl(ggsn_ip); + if (gsna_to_osa(&gtp_addr, &pdp->lib->gsnru) < 0) { + LOGPDPCTXP(LOGL_INFO, pdp, "Activate RAB: rab_id=%u, teid_gn=%x: Invalid GTP1U address! len=%u\n", + rab_id, pdp->lib->teid_gn, pdp->lib->gsnru.l); + return -EINVAL; + }

- LOGPDPCTXP(LOGL_INFO, pdp, "Activate RAB: rab_id=%u, ggsn_ip=%x, teid_gn=%x\n", - rab_id, ggsn_ip, pdp->lib->teid_gn); + LOGPDPCTXP(LOGL_INFO, pdp, "Activate RAB: rab_id=%u, ggsn_ip=%s, teid_gn=%x\n", + rab_id, osmo_sockaddr_ntop(&gtp_addr.u.sa, ip_str), pdp->lib->teid_gn);

- return sgsn_ranap_iu_tx_rab_ps_ass_req(ue_ctx, rab_id, ggsn_ip, pdp->lib->teid_gn); + return sgsn_ranap_iu_tx_rab_ps_ass_req(ue_ctx, rab_id, &gtp_addr, pdp->lib->teid_gn); }

int sgsn_pdp_ctx_iu_rab_deactivate(struct sgsn_pdp_ctx *pdp, uint8_t rab_id)