openbsc[master]: OML: fix potential OOB memory access

2017-06-12 Thread Neels Hofmeyr

Patch Set 1: Code-Review+2

(1 comment)

https://gerrit.osmocom.org/#/c/2885/1/openbsc/src/libbsc/abis_nm.c
File openbsc/src/libbsc/abis_nm.c:

Line 487:   if (m_id_len > MAX_BTS_FEATURES/8 + 1) {
You also need to drop this +1, right?


-- 
To view, visit https://gerrit.osmocom.org/2885
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib107daa6e8b9bc397a10756071849f8ff82455d5
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 
Gerrit-HasComments: Yes


[MERGED] openbsc[master]: OML: fix potential OOB memory access

2017-06-12 Thread Neels Hofmeyr
Neels Hofmeyr has submitted this change and it was merged.

Change subject: OML: fix potential OOB memory access
..


OML: fix potential OOB memory access

Use sizeof target BTS feature storage to make sure we always fit into
pre-allocated memory. Also use it for log check.

Change-Id: Ib107daa6e8b9bc397a10756071849f8ff82455d5
Fixes: CID 170581
---
M openbsc/src/libbsc/abis_nm.c
1 file changed, 2 insertions(+), 2 deletions(-)

Approvals:
  Neels Hofmeyr: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 551c0bf..1715688 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -490,13 +490,13 @@
m_id_len = MAX_BTS_FEATURES/8;
}
 
-   if (m_id_len > _NUM_BTS_FEAT/8 + 1)
+   if (m_id_len > sizeof(bts->_features_data))
LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: 
reported unexpectedly long (%u bytes) "
 "feature vector - most likely it was compiled 
against newer BSC headers. "
 "Consider upgrading your BSC to later version.\n",
 bts->nr, m_id_len);
 
-   memcpy(bts->_features_data, TLVP_VAL(&tp, NM_ATT_MANUF_ID), 
m_id_len);
+   memcpy(bts->_features_data, TLVP_VAL(&tp, NM_ATT_MANUF_ID), 
sizeof(bts->_features_data));
adjust = m_id_len + 3; /* adjust for parsed TL16V struct */
 
for (i = 0; i < _NUM_BTS_FEAT; i++)

-- 
To view, visit https://gerrit.osmocom.org/2885
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib107daa6e8b9bc397a10756071849f8ff82455d5
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Neels Hofmeyr 


[PATCH] openbsc[master]: OML: fix potential OOB memory access

2017-06-12 Thread Max

Review at  https://gerrit.osmocom.org/2885

OML: fix potential OOB memory access

Use sizeof target BTS feature storage to make sure we always fit into
pre-allocated memory. Also use it for log check.

Change-Id: Ib107daa6e8b9bc397a10756071849f8ff82455d5
Fixes: CID 170581
---
M openbsc/src/libbsc/abis_nm.c
1 file changed, 2 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.osmocom.org:29418/openbsc refs/changes/85/2885/1

diff --git a/openbsc/src/libbsc/abis_nm.c b/openbsc/src/libbsc/abis_nm.c
index 551c0bf..1715688 100644
--- a/openbsc/src/libbsc/abis_nm.c
+++ b/openbsc/src/libbsc/abis_nm.c
@@ -490,13 +490,13 @@
m_id_len = MAX_BTS_FEATURES/8;
}
 
-   if (m_id_len > _NUM_BTS_FEAT/8 + 1)
+   if (m_id_len > sizeof(bts->_features_data))
LOGP(DNM, LOGL_NOTICE, "BTS%u Get Attributes Response: 
reported unexpectedly long (%u bytes) "
 "feature vector - most likely it was compiled 
against newer BSC headers. "
 "Consider upgrading your BSC to later version.\n",
 bts->nr, m_id_len);
 
-   memcpy(bts->_features_data, TLVP_VAL(&tp, NM_ATT_MANUF_ID), 
m_id_len);
+   memcpy(bts->_features_data, TLVP_VAL(&tp, NM_ATT_MANUF_ID), 
sizeof(bts->_features_data));
adjust = m_id_len + 3; /* adjust for parsed TL16V struct */
 
for (i = 0; i < _NUM_BTS_FEAT; i++)

-- 
To view, visit https://gerrit.osmocom.org/2885
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib107daa6e8b9bc397a10756071849f8ff82455d5
Gerrit-PatchSet: 1
Gerrit-Project: openbsc
Gerrit-Branch: master
Gerrit-Owner: Max