[PATCH 2/7] src: more robust ipaccess_idtag_parse()

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.

pablo at gnumonks.org pablo at gnumonks.org
Mon Apr 11 15:04:25 UTC 2011


From: Pablo Neira Ayuso <pablo at gnumonks.org>

Now ipaccess_idtag_parse() returns -EINVAL instead of -1. We also
check for the return value of this function in every invocation to
skip further processing in case of messages with malformed TLVs.

This idea was suggested by Zecke.
---
 openbsc/src/libabis/input/ipaccess.c |   14 +++++++++-----
 openbsc/src/osmo-bsc_nat/bsc_nat.c   |    8 +++++++-
 openbsc/src/osmo-bsc_nat/bsc_ussd.c  |    8 +++++++-
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/openbsc/src/libabis/input/ipaccess.c b/openbsc/src/libabis/input/ipaccess.c
index c4c13d7..ba641a8 100644
--- a/openbsc/src/libabis/input/ipaccess.c
+++ b/openbsc/src/libabis/input/ipaccess.c
@@ -123,7 +123,7 @@ int ipaccess_idtag_parse(struct tlv_parsed *dec, unsigned char *buf, int len)
 
 		if (t_len > len + 1) {
 			LOGP(DMI, LOGL_ERROR, "The tag does not fit: %d\n", t_len);
-			return -1;
+			return -EINVAL;
 		}
 
 		DEBUGPC(DMI, "%s='%s' ", ipaccess_idtag_name(t_tag), cur);
@@ -251,7 +251,7 @@ static int ipaccess_rcvmsg(struct e1inp_line *line, struct msgb *msg,
 	u_int16_t site_id = 0, bts_id = 0, trx_id = 0;
 	struct gsm_bts *bts;
 	char *unitid;
-	int len;
+	int len, ret;
 
 	/* handle base messages */
 	ipaccess_rcvmsg_base(msg, bfd);
@@ -260,10 +260,14 @@ static int ipaccess_rcvmsg(struct e1inp_line *line, struct msgb *msg,
 	case IPAC_MSGT_ID_RESP:
 		DEBUGP(DMI, "ID_RESP ");
 		/* parse tags, search for Unit ID */
-		ipaccess_idtag_parse(&tlvp, (u_int8_t *)msg->l2h + 2,
-				 msgb_l2len(msg)-2);
+		ret = ipaccess_idtag_parse(&tlvp, (u_int8_t *)msg->l2h + 2,
+					   msgb_l2len(msg)-2);
 		DEBUGP(DMI, "\n");
-
+		if (ret < 0) {
+			LOGP(DINP, LOGL_ERROR, "ignoring IPA response message "
+					       "with malformed TLVs\n");
+			return ret;
+		}
 		if (!TLVP_PRESENT(&tlvp, IPAC_IDTAG_UNIT))
 			break;
 
diff --git a/openbsc/src/osmo-bsc_nat/bsc_nat.c b/openbsc/src/osmo-bsc_nat/bsc_nat.c
index 7586294..8c164a2 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_nat.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_nat.c
@@ -1019,9 +1019,15 @@ exit:
 		/* do we know who is handling this? */
 		if (msg->l2h[0] == IPAC_MSGT_ID_RESP) {
 			struct tlv_parsed tvp;
-			ipaccess_idtag_parse(&tvp,
+			int ret;
+			ret = ipaccess_idtag_parse(&tvp,
 					     (unsigned char *) msg->l2h + 2,
 					     msgb_l2len(msg) - 2);
+			if (ret < 0) {
+				LOGP(DNAT, LOGL_ERROR, "ignoring IPA response "
+					"message with malformed TLVs\n");
+				return ret;
+			}
 			if (TLVP_PRESENT(&tvp, IPAC_IDTAG_UNITNAME))
 				ipaccess_auth_bsc(&tvp, bsc);
 		}
diff --git a/openbsc/src/osmo-bsc_nat/bsc_ussd.c b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
index ff1d27a..4beef7b 100644
--- a/openbsc/src/osmo-bsc_nat/bsc_ussd.c
+++ b/openbsc/src/osmo-bsc_nat/bsc_ussd.c
@@ -123,9 +123,15 @@ static int ussd_read_cb(struct bsc_fd *bfd)
 	if (hh->proto == IPAC_PROTO_IPACCESS) {
 		if (msg->l2h[0] == IPAC_MSGT_ID_RESP) {
 			struct tlv_parsed tvp;
-			ipaccess_idtag_parse(&tvp,
+			int ret;
+			ret = ipaccess_idtag_parse(&tvp,
 					     (unsigned char *) msg->l2h + 2,
 					     msgb_l2len(msg) - 2);
+			if (ret < 0) {
+				LOGP(DNAT, LOGL_ERROR, "ignoring IPA response "
+					"message with malformed TLVs\n");
+				return ret;
+			}
 			if (TLVP_PRESENT(&tvp, IPAC_IDTAG_UNITNAME))
 				ussd_auth_con(&tvp, conn);
 		}
-- 
1.7.2.3





More information about the OpenBSC mailing list