forked from osmocom/wireshark
protobuf: Do not crash on zero length bytes element
If a field name has been written to the json dumper for a bytes element (Base64), then a Base64 value must be written later, even if the value is zero length. Move the JSON_DUMPER_FLAGS_NO_DEBUG flag to the json_dumper header, and use it in the protobuf dissector, so that errors in the JSON dumper state transitions do not abort the application through a ws_error() call. Use DISSECTOR_ASSERT in that case, since it should happen only with a dissector bug (as with the zero bytes elements issue fixed here), not with malformed packets. Only instantiate the json_dumper and create its output string if we intend on displaying its output, instead of doing so whenever we have a message type name. Fix #18730.
This commit is contained in:
parent
ae14849864
commit
5b96d57b18
|
@ -698,13 +698,13 @@ protobuf_dissect_field_value(proto_tree *value_tree, tvbuff_t *tvb, guint offset
|
||||||
|
|
||||||
case PROTOBUF_TYPE_BYTES:
|
case PROTOBUF_TYPE_BYTES:
|
||||||
if (field_desc && dumper) {
|
if (field_desc && dumper) {
|
||||||
|
json_dumper_begin_base64(dumper);
|
||||||
buf = (char*) tvb_memdup(wmem_file_scope(), tvb, offset, length);
|
buf = (char*) tvb_memdup(wmem_file_scope(), tvb, offset, length);
|
||||||
if (buf) {
|
if (buf) {
|
||||||
json_dumper_begin_base64(dumper);
|
|
||||||
json_dumper_write_base64(dumper, buf, length);
|
json_dumper_write_base64(dumper, buf, length);
|
||||||
json_dumper_end_base64(dumper);
|
|
||||||
wmem_free(wmem_file_scope(), buf);
|
wmem_free(wmem_file_scope(), buf);
|
||||||
}
|
}
|
||||||
|
json_dumper_end_base64(dumper);
|
||||||
}
|
}
|
||||||
if (field_dissector) {
|
if (field_dissector) {
|
||||||
if (!show_details) { /* don't show Value node if there is a subdissector for this field */
|
if (!show_details) { /* don't show Value node if there is a subdissector for this field */
|
||||||
|
@ -1528,10 +1528,6 @@ dissect_protobuf(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data
|
||||||
const PbwDescriptor* message_desc = NULL;
|
const PbwDescriptor* message_desc = NULL;
|
||||||
const gchar* data_str = NULL;
|
const gchar* data_str = NULL;
|
||||||
gchar *json_str, *p, *q;
|
gchar *json_str, *p, *q;
|
||||||
json_dumper dumper = {
|
|
||||||
.output_string = (display_json_mapping ? g_string_new(NULL) : NULL),
|
|
||||||
.flags = JSON_DUMPER_FLAGS_PRETTY_PRINT,
|
|
||||||
};
|
|
||||||
|
|
||||||
/* initialize only the first time the protobuf dissector is called */
|
/* initialize only the first time the protobuf dissector is called */
|
||||||
if (!protobuf_dissector_called) {
|
if (!protobuf_dissector_called) {
|
||||||
|
@ -1620,10 +1616,19 @@ dissect_protobuf(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data
|
||||||
message_desc = find_message_type_by_udp_port(pinfo);
|
message_desc = find_message_type_by_udp_port(pinfo);
|
||||||
}
|
}
|
||||||
|
|
||||||
dissect_protobuf_message(tvb, offset, tvb_reported_length_remaining(tvb, offset), pinfo,
|
if (display_json_mapping && message_desc) {
|
||||||
protobuf_tree, message_desc, -1, pinfo->ptype == PT_UDP, (message_desc ? &dumper : NULL), NULL, NULL);
|
json_dumper dumper = {
|
||||||
|
.output_string = g_string_new(NULL),
|
||||||
|
.flags = JSON_DUMPER_FLAGS_PRETTY_PRINT | JSON_DUMPER_FLAGS_NO_DEBUG,
|
||||||
|
};
|
||||||
|
|
||||||
if (display_json_mapping && message_desc && json_dumper_finish(&dumper)) {
|
/* Dissecting can throw an exception, ideally CLEANUP_PUSH and _POP
|
||||||
|
* should be used to free the GString to avoid a leak.
|
||||||
|
*/
|
||||||
|
dissect_protobuf_message(tvb, offset, tvb_reported_length_remaining(tvb, offset), pinfo,
|
||||||
|
protobuf_tree, message_desc, -1, pinfo->ptype == PT_UDP, &dumper, NULL, NULL);
|
||||||
|
|
||||||
|
DISSECTOR_ASSERT_HINT(json_dumper_finish(&dumper), "Bad json_dumper state");
|
||||||
ti = proto_tree_add_item(tree, proto_protobuf_json_mapping, tvb, 0, -1, ENC_NA);
|
ti = proto_tree_add_item(tree, proto_protobuf_json_mapping, tvb, 0, -1, ENC_NA);
|
||||||
protobuf_json_tree = proto_item_add_subtree(ti, ett_protobuf_json);
|
protobuf_json_tree = proto_item_add_subtree(ti, ett_protobuf_json);
|
||||||
|
|
||||||
|
@ -1644,6 +1649,9 @@ dissect_protobuf(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data
|
||||||
|
|
||||||
g_free(json_str);
|
g_free(json_str);
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
dissect_protobuf_message(tvb, offset, tvb_reported_length_remaining(tvb, offset), pinfo,
|
||||||
|
protobuf_tree, message_desc, -1, pinfo->ptype == PT_UDP, NULL, NULL, NULL);
|
||||||
}
|
}
|
||||||
|
|
||||||
return tvb_captured_length(tvb);
|
return tvb_captured_length(tvb);
|
||||||
|
|
|
@ -34,7 +34,6 @@ enum json_dumper_element_type {
|
||||||
#define JSON_DUMPER_HAS_NAME (1 << 3)
|
#define JSON_DUMPER_HAS_NAME (1 << 3)
|
||||||
|
|
||||||
#define JSON_DUMPER_FLAGS_ERROR (1 << 16) /* Output flag: an error occurred. */
|
#define JSON_DUMPER_FLAGS_ERROR (1 << 16) /* Output flag: an error occurred. */
|
||||||
#define JSON_DUMPER_FLAGS_NO_DEBUG (1 << 17) /* Input flag: disable debug prints (intended for speeding up fuzzing). */
|
|
||||||
|
|
||||||
enum json_dumper_change {
|
enum json_dumper_change {
|
||||||
JSON_DUMPER_BEGIN,
|
JSON_DUMPER_BEGIN,
|
||||||
|
|
|
@ -55,6 +55,7 @@ typedef struct json_dumper {
|
||||||
GString *output_string; /**< Output GLib strings. If it is not NULL, JSON will be dumped in the string. */
|
GString *output_string; /**< Output GLib strings. If it is not NULL, JSON will be dumped in the string. */
|
||||||
#define JSON_DUMPER_FLAGS_PRETTY_PRINT (1 << 0) /* Enable pretty printing. */
|
#define JSON_DUMPER_FLAGS_PRETTY_PRINT (1 << 0) /* Enable pretty printing. */
|
||||||
#define JSON_DUMPER_DOT_TO_UNDERSCORE (1 << 1) /* Convert dots to underscores in keys */
|
#define JSON_DUMPER_DOT_TO_UNDERSCORE (1 << 1) /* Convert dots to underscores in keys */
|
||||||
|
#define JSON_DUMPER_FLAGS_NO_DEBUG (1 << 17) /* Disable fatal ws_error messsges on error(intended for speeding up fuzzing). */
|
||||||
int flags;
|
int flags;
|
||||||
/* for internal use, initialize with zeroes. */
|
/* for internal use, initialize with zeroes. */
|
||||||
int current_depth;
|
int current_depth;
|
||||||
|
|
Loading…
Reference in New Issue