aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorGuy Harris <guy@alum.mit.edu>2018-04-12 17:56:51 -0700
committerGuy Harris <guy@alum.mit.edu>2018-04-13 00:57:32 +0000
commit05615015057f13197d5694c426bccccfa7de5f5f (patch)
treeee5c350eb0c398c3f4986a33c87feec173ffe02b
parent2a0ba8fea220c13ab90491eb7c4f8c0ea303a2e8 (diff)
Fix the length of the payload of a private_1 or audio PES packet.
The length field's value doesn't include the length of the length field itself. Change-Id: Icd0cc2721a32212296929d248b9305b0f4a051e6 Reviewed-on: https://code.wireshark.org/review/26920 Reviewed-by: Guy Harris <guy@alum.mit.edu>
-rw-r--r--epan/dissectors/asn1/mpeg-pes/packet-mpeg-pes-template.c61
-rw-r--r--epan/dissectors/packet-mpeg-pes.c71
2 files changed, 111 insertions, 21 deletions
diff --git a/epan/dissectors/asn1/mpeg-pes/packet-mpeg-pes-template.c b/epan/dissectors/asn1/mpeg-pes/packet-mpeg-pes-template.c
index 5a09ebc..fb10b5b 100644
--- a/epan/dissectors/asn1/mpeg-pes/packet-mpeg-pes-template.c
+++ b/epan/dissectors/asn1/mpeg-pes/packet-mpeg-pes-template.c
@@ -12,9 +12,10 @@
#include "config.h"
#include <epan/packet.h>
-#include <wiretap/wtap.h>
#include <epan/asn1.h>
+#include <wiretap/wtap.h>
+
#include "packet-per.h"
#include "packet-mpeg-pes-hf.c"
@@ -69,6 +70,11 @@ static int hf_mpeg_video_data = -1;
static dissector_handle_t mpeg_handle;
enum { PES_PREFIX = 1 };
+/*
+ * XXX - some of these aren't documented in ISO/IEC 13818-1:2007;
+ * Table 2-22 "Stream_id assignments" starts with 0xbc, and also
+ * lists some types not given here. Where are the others described?
+ */
enum {
STREAM_PICTURE = 0x00,
STREAM_SEQUENCE = 0xb3,
@@ -452,18 +458,57 @@ dissect_mpeg_pes(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data
if ((tvb_get_guint8(tvb, 6) & 0xc0) == 0x80) {
int header_length;
tvbuff_t *es;
+ int save_offset = offset;
offset = dissect_mpeg_pes_Stream(tvb, offset, &asn1_ctx,
tree, hf_mpeg_pes_extension);
/* https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2229
- * A value of 0 indicates that the PES packet length is neither specified nor
- * bounded and is allowed only in PES packets whose payload is a video elementary
- * stream contained in Transport Stream packets.
- * XXX Some one with access to the spec should check this
+ * A value of 0 indicates that the PES packet length
+ * is neither specified nor bounded and is allowed
+ * only in PES packets whose payload is a video
+ * elementary stream contained in Transport Stream
+ * packets.
+ *
+ * See ISO/IEC 13818-1:2007, section 2.4.3.7
+ * "Semantic definition of fields in PES packet",
+ * which says of the PES_packet_length that "A value
+ * of 0 indicates that the PES packet length is
+ * neither specified nor bounded and is allowed only
+ * in PES packets whose payload consists of bytes
+ * from a video elementary stream contained in
+ * Transport Stream packets."
*/
- if(length !=0 && stream != STREAM_VIDEO){
- length -= 5;
- }
+ if(length !=0 && stream != STREAM_VIDEO){
+ /*
+ * XXX - note that ISO/IEC 13818-1:2007
+ * says that the length field is *not*
+ * part of the above extension.
+ *
+ * This means that the length of the length
+ * field itself should *not* be subtracted
+ * from the length field; ISO/IEC 13818-1:2007
+ * says that the PES_packet_length field is
+ * "A 16-bit field specifying the number of
+ * bytes in the PES packet following the
+ * last byte of the field."
+ *
+ * So we calculate the size of the extension,
+ * in bytes, by subtracting the saved bit
+ * offset value from the current bit offset
+ * value, divide by 8 to convert to a size
+ * in bytes, and then subtract 2 to remove
+ * the length field's length from the total
+ * length.
+ *
+ * (In addition, ISO/IEC 13818-1:2007
+ * suggests that the length field is
+ * always present, but this code, when
+ * processing some stream ID types, doesn't
+ * treat it as being present. Where are
+ * the formats of those payloads specified?)
+ */
+ length -= ((offset - save_offset) / 8) - 2;
+ }
header_length = tvb_get_guint8(tvb, 8);
if (header_length > 0) {
diff --git a/epan/dissectors/packet-mpeg-pes.c b/epan/dissectors/packet-mpeg-pes.c
index e9539d7..538c853 100644
--- a/epan/dissectors/packet-mpeg-pes.c
+++ b/epan/dissectors/packet-mpeg-pes.c
@@ -20,9 +20,10 @@
#include "config.h"
#include <epan/packet.h>
-#include <wiretap/wtap.h>
#include <epan/asn1.h>
+#include <wiretap/wtap.h>
+
#include "packet-per.h"
@@ -80,7 +81,7 @@ static int hf_mpeg_pes_frame_type = -1; /* T_frame_type */
static int hf_mpeg_pes_vbv_delay = -1; /* BIT_STRING_SIZE_16 */
/*--- End of included file: packet-mpeg-pes-hf.c ---*/
-#line 21 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
+#line 22 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
/*--- Included file: packet-mpeg-pes-ett.c ---*/
#line 1 "./asn1/mpeg-pes/packet-mpeg-pes-ett.c"
@@ -92,7 +93,7 @@ static gint ett_mpeg_pes_Group_of_pictures = -1;
static gint ett_mpeg_pes_Picture = -1;
/*--- End of included file: packet-mpeg-pes-ett.c ---*/
-#line 22 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
+#line 23 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
/*--- Included file: packet-mpeg-pes-fn.c ---*/
#line 1 "./asn1/mpeg-pes/packet-mpeg-pes-fn.c"
@@ -460,7 +461,7 @@ dissect_mpeg_pes_Picture(tvbuff_t *tvb _U_, int offset _U_, asn1_ctx_t *actx _U_
/*--- End of included file: packet-mpeg-pes-fn.c ---*/
-#line 23 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
+#line 24 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
void proto_register_mpeg_pes(void);
void proto_reg_handoff_mpeg_pes(void);
@@ -510,6 +511,11 @@ static int hf_mpeg_video_data = -1;
static dissector_handle_t mpeg_handle;
enum { PES_PREFIX = 1 };
+/*
+ * XXX - some of these aren't documented in ISO/IEC 13818-1:2007;
+ * Table 2-22 "Stream_id assignments" starts with 0xbc, and also
+ * lists some types not given here. Where are the others described?
+ */
enum {
STREAM_PICTURE = 0x00,
STREAM_SEQUENCE = 0xb3,
@@ -893,18 +899,57 @@ dissect_mpeg_pes(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data
if ((tvb_get_guint8(tvb, 6) & 0xc0) == 0x80) {
int header_length;
tvbuff_t *es;
+ int save_offset = offset;
offset = dissect_mpeg_pes_Stream(tvb, offset, &asn1_ctx,
tree, hf_mpeg_pes_extension);
/* https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2229
- * A value of 0 indicates that the PES packet length is neither specified nor
- * bounded and is allowed only in PES packets whose payload is a video elementary
- * stream contained in Transport Stream packets.
- * XXX Some one with access to the spec should check this
+ * A value of 0 indicates that the PES packet length
+ * is neither specified nor bounded and is allowed
+ * only in PES packets whose payload is a video
+ * elementary stream contained in Transport Stream
+ * packets.
+ *
+ * See ISO/IEC 13818-1:2007, section 2.4.3.7
+ * "Semantic definition of fields in PES packet",
+ * which says of the PES_packet_length that "A value
+ * of 0 indicates that the PES packet length is
+ * neither specified nor bounded and is allowed only
+ * in PES packets whose payload consists of bytes
+ * from a video elementary stream contained in
+ * Transport Stream packets."
*/
- if(length !=0 && stream != STREAM_VIDEO){
- length -= 5;
- }
+ if(length !=0 && stream != STREAM_VIDEO){
+ /*
+ * XXX - note that ISO/IEC 13818-1:2007
+ * says that the length field is *not*
+ * part of the above extension.
+ *
+ * This means that the length of the length
+ * field itself should *not* be subtracted
+ * from the length field; ISO/IEC 13818-1:2007
+ * says that the PES_packet_length field is
+ * "A 16-bit field specifying the number of
+ * bytes in the PES packet following the
+ * last byte of the field."
+ *
+ * So we calculate the size of the extension,
+ * in bytes, by subtracting the saved bit
+ * offset value from the current bit offset
+ * value, divide by 8 to convert to a size
+ * in bytes, and then subtract 2 to remove
+ * the length field's length from the total
+ * length.
+ *
+ * (In addition, ISO/IEC 13818-1:2007
+ * suggests that the length field is
+ * always present, but this code, when
+ * processing some stream ID types, doesn't
+ * treat it as being present. Where are
+ * the formats of those payloads specified?)
+ */
+ length -= ((offset - save_offset) / 8) - 2;
+ }
header_length = tvb_get_guint8(tvb, 8);
if (header_length > 0) {
@@ -1174,7 +1219,7 @@ proto_register_mpeg_pes(void)
"BIT_STRING_SIZE_16", HFILL }},
/*--- End of included file: packet-mpeg-pes-hfarr.c ---*/
-#line 532 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
+#line 577 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
{ &hf_mpeg_pes_pack_header,
{ "Pack header", "mpeg-pes.pack",
FT_NONE, BASE_NONE, NULL, 0, NULL, HFILL }},
@@ -1292,7 +1337,7 @@ proto_register_mpeg_pes(void)
&ett_mpeg_pes_Picture,
/*--- End of included file: packet-mpeg-pes-ettarr.c ---*/
-#line 639 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
+#line 684 "./asn1/mpeg-pes/packet-mpeg-pes-template.c"
&ett_mpeg_pes_pack_header,
&ett_mpeg_pes_header_data,
&ett_mpeg_pes_trick_mode