From f6ef53e3ed87cb83144e2e7270f38a459d459711 Mon Sep 17 00:00:00 2001 From: Pau Espin Pedrol Date: Wed, 25 Mar 2020 13:50:09 +0100 Subject: 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 Petri-Dish: Pascal Quantin Tested-by: Petri Dish Buildbot Reviewed-by: Pascal Quantin --- epan/dissectors/packet-csn1.c | 18 ++++++++++++++++-- epan/dissectors/packet-csn1.h | 6 +++--- 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: ::= { 1 } ** 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: ::= { 1 } ** 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 * ::= {1 } ** 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 * ::= { 0 } ** 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) -- cgit v1.2.3