aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorPau Espin Pedrol <pespin@sysmocom.de>2020-03-25 13:50:09 +0100
committerPascal Quantin <pascal@wireshark.org>2020-03-25 16:51:35 +0000
commitf6ef53e3ed87cb83144e2e7270f38a459d459711 (patch)
treebc6228100f00953e33c05b2d43342a6583ed2de3
parent7b8ea03c6472c0f52b17e13324826839eb871dd3 (diff)
csn1: Validate recursive array max size during decoding
This way if CSN1 encoded bitstream contains more elements than what the defintion expects it will fail instead of overflowing the decoded buffer. Example: RA Capabilities struct (recursive array) sent by a real android phone when attaching to the network. Then SGSN sends it back and osmo-pcu would crash similar to this: *** stack smashing detected ***: terminated Process terminating with default action of signal 6 (SIGABRT): dumping core at 0x4C62CE5: raise (in /usr/lib/libc-2.31.so) by 0x4C4C856: abort (in /usr/lib/libc-2.31.so) by 0x4CA62AF: __libc_message (in /usr/lib/libc-2.31.so) by 0x4D36069: __fortify_fail (in /usr/lib/libc-2.31.so) by 0x4D36033: __stack_chk_fail (in /usr/lib/libc-2.31.so) by 0x124706: testRAcap2(void*) (RLCMACTest.cpp:468) Port from osmo-pcu.git efad80bfbffb2a35d2516e56dc40979f19c6c370 Related: https://osmocom.org/issues/4463 Change-Id: I6bdd6960141829491aebbfdaab548c41d4a3bc9f Reviewed-on: https://code.wireshark.org/review/36572 Reviewed-by: Harald Welte <laforge@gnumonks.org> Petri-Dish: Pascal Quantin <pascal@wireshark.org> Tested-by: Petri Dish Buildbot Reviewed-by: Pascal Quantin <pascal@wireshark.org>
-rw-r--r--epan/dissectors/packet-csn1.c18
-rw-r--r--epan/dissectors/packet-csn1.h6
2 files changed, 19 insertions, 5 deletions
diff --git a/epan/dissectors/packet-csn1.c b/epan/dissectors/packet-csn1.c
index 3541d72849..616e80ffc7 100644
--- a/epan/dissectors/packet-csn1.c
+++ b/epan/dissectors/packet-csn1.c
@@ -1332,9 +1332,10 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
case CSN_RECURSIVE_TARRAY:
{ /* Recursive way to specify an array of type: <lists> ::= { 1 <type> } ** 0 ;
* M_REC_TARRAY(_STRUCT, _MEMBER, _MEMBER_TYPE, _ElementCountField)
- * {t, offsetof(_STRUCT, _ElementCountField), (void*)CSNDESCR_##_MEMBER_TYPE, offsetof(_STRUCT, _MEMBER), #_MEMBER, (StreamSerializeFcn_t)sizeof(_MEMBER_TYPE)}
+ * {t, offsetof(_STRUCT, _ElementCountField), (void*)CSNDESCR_##_MEMBER_TYPE, offsetof(_STRUCT, _MEMBER), #_MEMBER, (StreamSerializeFcn_t)sizeof(_MEMBER_TYPE), NULL, NULL, (void_fn_t)ElementsOf(((_STRUCT*)0)->_MEMBER)}
*/
gint16 nSizeElement = (gint16)(gint32)pDescr->value;
+ guint32 nSizeArray = (guint32)((guintptr)pDescr->aux_fn);
guint8 ElementCount = 0;
while (existNextElement(tvb, bit_offset, Tag))
@@ -1346,6 +1347,12 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
remaining_bits_len--;
ElementCount++;
+ if (ElementCount > nSizeArray)
+ {
+ /* error: too many elements in recursive array. Increase its size! */
+ return ProcessError(tree , ar->pinfo, tvb, bit_offset, CSN_ERROR_STREAM_NOT_SUPPORTED, &ei_csn1_stream_not_supported, pDescr);
+ }
+
{ /* unpack the following data structure */
csnStream_t arT = *ar;
gint16 Status;
@@ -1400,9 +1407,10 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
case CSN_RECURSIVE_TARRAY_1:
{ /* Recursive way to specify an array of type: <lists> ::= <type> { 1 <type> } ** 0 ;
* M_REC_TARRAY(_STRUCT, _MEMBER, _MEMBER_TYPE, _ElementCountField)
- * {t, offsetof(_STRUCT, _ElementCountField), (void*)CSNDESCR_##_MEMBER_TYPE, offsetof(_STRUCT, _MEMBER), #_MEMBER, (StreamSerializeFcn_t)sizeof(_MEMBER_TYPE)}
+ * {t, offsetof(_STRUCT, _ElementCountField), (void*)CSNDESCR_##_MEMBER_TYPE, offsetof(_STRUCT, _MEMBER), #_MEMBER, (StreamSerializeFcn_t)sizeof(_MEMBER_TYPE), NULL, NULL, (void_fn_t)ElementsOf(((_STRUCT*)0)->_MEMBER)}
*/
gint16 nSizeElement = (gint16)(gint32)pDescr->value;
+ guint32 nSizeArray = (guint32)((guintptr)pDescr->aux_fn);
guint8 ElementCount = 0;
csnStream_t arT = *ar;
gboolean EndOfList = FALSE;
@@ -1414,6 +1422,12 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
{ /* get data element */
ElementCount++;
+ if (ElementCount >= nSizeArray)
+ {
+ /* error: too many elements in recursive array. Increase its size! */
+ return ProcessError(tree , ar->pinfo, tvb, bit_offset, CSN_ERROR_STREAM_NOT_SUPPORTED, &ei_csn1_stream_not_supported, pDescr);
+ }
+
test_tree = proto_tree_add_subtree_format(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, "%s[%d]", pDescr->sz, ElementCount-1);
csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
diff --git a/epan/dissectors/packet-csn1.h b/epan/dissectors/packet-csn1.h
index e028dea451..304a01e7bd 100644
--- a/epan/dissectors/packet-csn1.h
+++ b/epan/dissectors/packet-csn1.h
@@ -448,7 +448,7 @@ gint16 csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pD
* Par4: will hold the number of element in the array after unpacking
*****************************************************************************/
#define M_REC_TARRAY(_STRUCT, _MEMBER, _MEMBER_TYPE, _ElementCountField, _HF_PTR)\
- {CSN_RECURSIVE_TARRAY, offsetof(_STRUCT, _ElementCountField), {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, sizeof(_MEMBER_TYPE), _HF_PTR, NULL, NULL}
+ {CSN_RECURSIVE_TARRAY, offsetof(_STRUCT, _ElementCountField), {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, sizeof(_MEMBER_TYPE), _HF_PTR, NULL, (void_fn_t)ElementsOf(((_STRUCT*)0)->_MEMBER)}
/******************************************************************************
* M_REC_TARRAY1(Par1, Par2, Par3, Par4)
@@ -456,7 +456,7 @@ gint16 csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pD
* <list> ::= <type> {1 <type>} ** 0 ;
*****************************************************************************/
#define M_REC_TARRAY_1(_STRUCT, _MEMBER, _MEMBER_TYPE, _ElementCountField, _HF_PTR)\
- {CSN_RECURSIVE_TARRAY_1, offsetof(_STRUCT, _ElementCountField), {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, sizeof(_MEMBER_TYPE), _HF_PTR, NULL, NULL}
+ {CSN_RECURSIVE_TARRAY_1, offsetof(_STRUCT, _ElementCountField), {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, sizeof(_MEMBER_TYPE), _HF_PTR, NULL, (void_fn_t)ElementsOf(((_STRUCT*)0)->_MEMBER)}
/******************************************************************************
* M_REC_TARRAY2(Par1, Par2, Par3, Par4)
@@ -464,7 +464,7 @@ gint16 csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pD
* <lists> ::= <type> { 0 <type> } ** 1 ;
*****************************************************************************/
#define M_REC_TARRAY_2(_STRUCT, _MEMBER, _MEMBER_TYPE, _ElementCountField)\
- {CSN_RECURSIVE_TARRAY_2, offsetof(_STRUCT, _ElementCountField), {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, sizeof(_MEMBER_TYPE), NULL, NULL, NULL}
+ {CSN_RECURSIVE_TARRAY_2, offsetof(_STRUCT, _ElementCountField), {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, sizeof(_MEMBER_TYPE), NULL, NULL, (void_fn_t)ElementsOf(((_STRUCT*)0)->_MEMBER)}
/******************************************************************************
* M_TYPE(Par1, Par2, Par3)