osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

csn1: Avoid failing if optional DownlinkDualCarrierCapability_r7 is missing 

All additional release fields in RadioAccesCapabilities are considered
optional, and the CSN_DESCR for Content_t already marks almost all as such,
except DownlinkDualCarrierCapability_r7.

It has been found that some MS transmits a MS RA Capability with a Length=61 bits
where the last bit in the buffer is setting the Exist bit for
DownlinkDualCarrierCapability_r7 as 1. Hence, the CSN1 decoder failed to
decode the whole message because it expected to keep reading there
despite there's no more bytes to read.

While this is could actually be considered an MS bug, let's relax our
expectancies and simply consider the case { 1 <end> } as it was { 0 },
and mark skip decoding DownlinkDualCarrierCapability_r7. That what
wireshark (packet-gsm_a_gsm.c) or pycrate do for instance.

This patch itself doesn't fix the problem where actually the Exist bit
is stored as 1 in the output decoded structure, but simply allows keep
ongoing with decoding until the end. This issue will be fixed in a
follow-up patch.

This patch is a port from patch fixing same issue in the osmo-pcu.git copy of
csn1 decoder:
https://git.osmocom.org/osmo-pcu/commit/?id=ebdc0d8c170ee2dbf23b19056d6c2d0ef316b3c2
This commit is contained in:
Pau Espin 2021-10-20 16:50:29 +02:00
parent 4d0537162b
commit 6ba9c7b918
3 changed files with 47 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
epan/dissectors/packet-csn1.c
View File

@ -531,25 +531,26 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
csnStream_t arT = *ar;
proto_item *ti;
proto_tree *test_tree;
test_tree = proto_tree_add_subtree_format(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, "%s", pDescr->sz);
csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR*)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
if (Status >= 0)
if (pDescr->may_be_null && remaining_bits_len == 0)
{
proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
remaining_bits_len = arT.remaining_bits_len;
bit_offset = arT.bit_offset;
pDescr++;
proto_tree_add_none_format(tree, hf_null_data, tvb, 0, 0, "[NULL data]: %s Not Present", pDescr->sz);
} else {
test_tree = proto_tree_add_subtree_format(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, "%s", pDescr->sz);
csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR*)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
if (Status >= 0)
{
proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
remaining_bits_len = arT.remaining_bits_len;
bit_offset = arT.bit_offset;
}
else
{
/* Has already been processed: ProcessError("csnStreamDissector", Status, pDescr); */
return Status;
}
}
else
{
/* Has already been processed: ProcessError("csnStreamDissector", Status, pDescr); */
return Status;
}
pDescr++;
break;
}
@ -1002,22 +1003,25 @@ csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pDescr, t
proto_item *ti;
proto_tree *test_tree;
test_tree = proto_tree_add_subtree(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, pDescr->sz);
csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR *)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
if (Status >= 0)
if (pDescr->may_be_null && remaining_bits_len == 0)
{
proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
remaining_bits_len = arT.remaining_bits_len;
bit_offset = arT.bit_offset;
pDescr++;
proto_tree_add_none_format(tree, hf_null_data, tvb, 0, 0, "[NULL data]: %s Not Present", pDescr->sz);
} else {
test_tree = proto_tree_add_subtree(tree, tvb, bit_offset>>3, 1, ett_csn1, &ti, pDescr->sz);
csnStreamInit(&arT, bit_offset, remaining_bits_len, ar->pinfo);
Status = csnStreamDissector(test_tree, &arT, (const CSN_DESCR *)pDescr->descr.ptr, tvb, pvDATA(data, pDescr->offset), ett_csn1);
if (Status >= 0)
{
proto_item_set_len(ti,((arT.bit_offset-1)>>3) - (bit_offset>>3)+1);
remaining_bits_len = arT.remaining_bits_len;
bit_offset = arT.bit_offset;
}
else
{ /* return error code Has already been processed: */
return Status;
}
}
else
{ /* return error code Has already been processed: */
return Status;
}
pDescr++;
break;
}

11
epan/dissectors/packet-csn1.h
View File

@ -477,6 +477,17 @@ gint16 csnStreamDissector(proto_tree *tree, csnStream_t* ar, const CSN_DESCR* pD
#define M_TYPE(_STRUCT, _MEMBER, _MEMBER_TYPE)\
{CSN_TYPE, 0, {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), FALSE, #_MEMBER, NULL, 0, NULL, NULL, NULL}
/******************************************************************************
* M_TYPE_OR_NULL(Par1, Par2, Par3)
* Similar to the M_TYPE except that not only the request set of bits but also the
* end of the message may be encountered when looking for the next element in
* the message.
* Covers the case {null | 0 | 1 < IE >}
*****************************************************************************/
#define M_TYPE_OR_NULL(_STRUCT, _MEMBER, _MEMBER_TYPE)\
{CSN_TYPE, 0, {(const void*)CSNDESCR_##_MEMBER_TYPE}, offsetof(_STRUCT, _MEMBER), TRUE, #_MEMBER, NULL, 0, NULL, NULL, NULL}
/******************************************************************************
* M_TYPE_LABEL(Par1, Par2, Par3, Par4)
* Same as M_TYPE but allows to define a custom string for the subtree

2
epan/dissectors/packet-gsm_rlcmac.c
View File

@ -2750,7 +2750,7 @@ CSN_DESCR_BEGIN (Content_t)
/* additions in release 7 */
M_UINT_OR_NULL (Content_t, DTM_Handover_Capability, 1, &hf_content_dtm_handover_capability),
M_NEXT_EXIST_OR_NULL(Content_t, Exist_DownlinkDualCarrierCapability_r7, 1, &hf_content_multislot_capability_reduction_for_dl_dual_carrier_exist),
M_TYPE (Content_t, DownlinkDualCarrierCapability_r7, DownlinkDualCarrierCapability_r7_t),
M_TYPE_OR_NULL (Content_t, DownlinkDualCarrierCapability_r7, DownlinkDualCarrierCapability_r7_t),
M_UINT_OR_NULL (Content_t, FlexibleTimeslotAssignment, 1, &hf_content_flexible_timeslot_assignment),
M_UINT_OR_NULL (Content_t, GAN_PS_HandoverCapability, 1, &hf_content_gan_ps_handover_capability),