Incorrect padding in BFCP decoder
Summary
Wireshark decoder for BFCP protocol incorrectly dissects padding in various frames. Padding is supposed to be either 1, 2 or 3 octets long.
Steps to reproduce
Decode BFCP frames
What is the current bug behavior?
Padding length is calculated correctly. However it's tree item offset and previous tree item length are hardcoded. E.g. the attribute UserDispayName with "Attribute length" = 10, the "Name" dissected as 7 octets long (instead of 8). Padding is 2, but it's first octet is the real last octet of the "Name". This causes the offset shift of -1 for the rest of the frame. Therefore subsequent attributes cannot be decoded.
What is the expected correct behavior?
For every BFCP attribute containing padding, the padding length should be calculated before preceding tree item. And used for said tree item length and the padding tree item offset.
E.g. in packet-bfcp.c instead:
proto_tree_add_item(bfcp_attr_tree, hf_bfcp_user_disp_name, tvb, offset, length-3, ENC_ASCII|ENC_NA);
offset = offset + length-3;
pad_len = length & 0x03;
if(pad_len != 0){
pad_len = 4 - pad_len;
proto_tree_add_text(bfcp_attr_tree, tvb, offset, pad_len, "Padding");
}
offset = offset + pad_len;
This could work:
pad_len = length & 0x03;
if(pad_len != 0){
pad_len = 4 - pad_len;
}
proto_tree_add_item(bfcp_attr_tree, hf_bfcp_user_disp_name, tvb, offset, length-pad_len, ENC_ASCII|ENC_NA);
offset = offset + length-pad_len;
if(pad_len != 0){
proto_tree_add_text(bfcp_attr_tree, tvb, offset, pad_len, "Padding");
offset = offset + pad_len;
}
Sample capture file
Relevant logs and/or screenshots
(Paste any relevant logs here)
(Paste any relevant screenshots here)
Build information
Version 4.0.2 (v4.0.2-0-g415456d13370).
Copyright 1998-2022 Gerald Combs <gerald@wireshark.org> and contributors.
This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software Foundation, either version 2 of the License, or (at your option) any later version. This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
Compiled (64-bit) using Microsoft Visual Studio 2022 (VC++ 14.32, build 31332), with GLib 2.72.3, with PCRE2, with zlib 1.2.12, with Qt 5.15.2, with libpcap, with Lua 5.2.4, with GnuTLS 3.6.3 and PKCS #11 support, with Gcrypt 1.10.1, with Kerberos (MIT), with MaxMind, with nghttp2 1.46.0, with brotli, with LZ4, with Zstandard, with Snappy, with libxml2 2.9.14, with libsmi 0.4.8, with QtMultimedia, with automatic updates using WinSparkle 0.5.7, with AirPcap, with SpeexDSP (using bundled resampler), with Minizip, with binary plugins.
Running on 64-bit Windows 11 (21H2), build 22000, with Intel(R) Core(TM) i7-8850H CPU @ 2.60GHz (with SSE4.2), with 32574 MB of physical memory, with GLib 2.72.3, with PCRE2 10.40 2022-04-14, with Qt 5.15.2, with Npcap version 0.9984, based on libpcap version 1.9.1, with c-ares 1.18.1, with GnuTLS 3.6.3, with Gcrypt 1.10.1, with nghttp2 1.46.0, with brotli 1.0.9, with LZ4 1.9.3, with Zstandard 1.5.2, with AirPcap 4.1.0 build 1622, with light display mode, without HiDPI, with LC_TYPE=English_United Kingdom.utf8, binary plugins supported.