osmith has submitted this change. (
https://gerrit.osmocom.org/c/osmo-bsc-nat/+/28582 )
Change subject: bssap_conn: fix missing length check
......................................................................
bssap_conn: fix missing length check
Check length of msg_new before trying to encode a copy of the IE into
it. The code path for IE_AOIP_TRASP_ADDR is different since it gets
replaced. With the libosmocore patch in Depends below the
gsm0808_enc_aoip_trasp_addr function checks the length.
Depends: libosmocore I632986b99d841abff0f14c6da65f030175f5c4a1
Fixes: Coverity CID#273004
Change-Id: I1fc4c81e139bab3d7d977ef9467f62d8088884db
---
M src/osmo-bsc-nat/bssap_conn.c
1 file changed, 17 insertions(+), 13 deletions(-)
Approvals:
Jenkins Builder: Verified
dexter: Looks good to me, but someone else must approve
fixeria: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/src/osmo-bsc-nat/bssap_conn.c b/src/osmo-bsc-nat/bssap_conn.c
index ec2a897..7d9791a 100644
--- a/src/osmo-bsc-nat/bssap_conn.c
+++ b/src/osmo-bsc-nat/bssap_conn.c
@@ -18,6 +18,7 @@
*/
#include "config.h"
+#include <errno.h>
#include <osmocom/core/msgb.h>
#include <osmocom/gsm/gsm0808.h>
#include <osmocom/sigtran/sccp_helpers.h>
@@ -31,7 +32,7 @@
struct msgb *msg_old = *msg;
const struct tlv_definition *def = gsm0808_att_tlvdef();
int ofs = 1; /* first byte is bssmap message type */
- int rc;
+ uint8_t tag;
msg_new = msgb_alloc_headroom(BSSMAP_MSG_SIZE, BSSMAP_MSG_HEADROOM,
talloc_get_name(msg_old));
OSMO_ASSERT(msg_new);
@@ -39,26 +40,25 @@
while (ofs < msgb_l3len(msg_old)) {
int rv;
- uint8_t tag;
const uint8_t *val;
- uint16_t len;
+ const uint8_t len_tl = 2;
+ uint16_t len_v;
- rv = tlv_parse_one(&tag, &len, &val, def, &msg_old->l3h[ofs],
msgb_l3len(msg_old) - ofs);
+ rv = tlv_parse_one(&tag, &len_v, &val, def, &msg_old->l3h[ofs],
msgb_l3len(msg_old) - ofs);
if (rv < 0) {
LOGP(DMAIN, LOGL_ERROR, "Failed to parse bssmap msg\n");
msgb_free(msg_new);
return rv;
}
- if (tag == GSM0808_IE_AOIP_TRASP_ADDR)
- rc = gsm0808_enc_aoip_trasp_addr(msg_new, ss);
- else
- rc = tlv_encode_one(msg_new, def->def[tag].type, tag,
len, val);
-
- if (rc < 0) {
- LOGP(DMAIN, LOGL_ERROR, "Failed to encode tag %d into
copy of bssmap msg\n", tag);
- msgb_free(msg_new);
- return rc;
+ if (tag == GSM0808_IE_AOIP_TRASP_ADDR) {
+ if (gsm0808_enc_aoip_trasp_addr(msg_new, ss) == 0)
+ goto enc_err;
+ } else {
+ if (len_tl + len_v > msgb_tailroom(msg_new))
+ goto enc_err;
+ if (tlv_encode_one(msg_new, def->def[tag].type, tag,
len_v, val) < 0)
+ goto enc_err;
}
ofs += rv;
@@ -68,6 +68,10 @@
msgb_free(msg_old);
*msg = msg_new;
return 0;
+enc_err:
+ LOGP(DMAIN, LOGL_ERROR, "Failed to encode tag %d into copy of bssmap
msg\n", tag);
+ msgb_free(msg_new);
+ return -EINVAL;
}
int bssmap_tx_assignment_failure(enum bsc_nat_net net, struct subscr_conn
*subscr_conn, enum gsm0808_cause cause)
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/28582
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc-nat
Gerrit-Branch: master
Gerrit-Change-Id: I1fc4c81e139bab3d7d977ef9467f62d8088884db
Gerrit-Change-Number: 28582
Gerrit-PatchSet: 5
Gerrit-Owner: osmith <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <[email protected]>
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
Gerrit-MessageType: merged
