Change in osmo-msc[master]: VLR: reject overlong IMSIs in ID RESP messages

[Harald Welte]

Harald Welte has submitted this change and it was merged. ( https://gerrit.osmocom.org/9739 )

Change subject: VLR: reject overlong IMSIs in ID RESP messages
......................................................................

VLR: reject overlong IMSIs in ID RESP messages

Overlong IMSIs in ID RESP messages were accepted and used in
truncated form.

Log an error when truncation occurs, and prevent truncated IMSIs
from being installed for a subscriber via ID RESP messages.
Other code paths leading to vlr_subscr_set_imsi() with truncated
IMSIs will only a leave a trail of log entries for now, because
vlr_subscr_set_imsi() is currently unable to return an error code.

Change-Id: I785c994f41a646d8d83d3d82f5a9ae6b572eb641
Related: OS#2864
---
M src/libvlr/vlr.c
1 file changed, 13 insertions(+), 2 deletions(-)

Approvals:
  Harald Welte: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/libvlr/vlr.c b/src/libvlr/vlr.c
index 29098b1..cff2e12 100644
--- a/src/libvlr/vlr.c
+++ b/src/libvlr/vlr.c
@@ -396,7 +396,13 @@
 {
 	if (!vsub)
 		return;
-	OSMO_STRLCPY_ARRAY(vsub->imsi, imsi);
+
+	if (OSMO_STRLCPY_ARRAY(vsub->imsi, imsi) >= sizeof(vsub->imsi)) {
+		LOGP(DVLR, LOGL_NOTICE, "IMSI was truncated: full IMSI=%s, truncated IMSI=%s\n",
+		       imsi, vsub->imsi);
+		/* XXX Set truncated IMSI anyway, we currently cannot return an error from here. */
+	}
+
 	vsub->id = atoll(vsub->imsi);
 	DEBUGP(DVLR, "set IMSI on subscriber; IMSI=%s id=%llu\n",
 	       vsub->imsi, vsub->id);
@@ -1062,10 +1068,15 @@
 	/* update the vlr_subscr with the given identity */
 	switch (mi_type) {
 	case GSM_MI_TYPE_IMSI:
-		if (vsub->imsi[0]
+		if (strlen(mi_string) >= sizeof(vsub->imsi)) {
+			LOGVSUBP(LOGL_ERROR, vsub, "IMSI in ID RESP too long (>%zu bytes): %s\n",
+				 sizeof(vsub->imsi) - 1, mi_string);
+			return -ENOSPC; /* ignore message; do not avance LU FSM */
+		} else if (vsub->imsi[0]
 		    && !vlr_subscr_matches_imsi(vsub, mi_string)) {
 			LOGVSUBP(LOGL_ERROR, vsub, "IMSI in ID RESP differs:"
 				 " %s\n", mi_string);
+			/* XXX Should we return an error, e.g. -EINVAL ? */
 		} else
 			vlr_subscr_set_imsi(vsub, mi_string);
 		break;

-- 
To view, visit https://gerrit.osmocom.org/9739
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I785c994f41a646d8d83d3d82f5a9ae6b572eb641
Gerrit-Change-Number: 9739
Gerrit-PatchSet: 2
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-CC: Pau Espin Pedrol <pespin at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180625/377ccbbc/attachment.htm>