pespin submitted this change.

View Change

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(-)

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)

To view, visit change 43260. To unsubscribe, or for help writing mail filters, visit settings.