Skip to content

Commit

Permalink
Make sure we don't create comment options longer than 65535 bytes.
Browse files Browse the repository at this point in the history
Check in both editcap and Wireshark to make sure that comments have
fewer than 65536 bytes before accepting them.

This shoudl fix #18235, although there should also be checks in
libwiretap to catch cases where the user interface code doesn't do the
check (it should be done in the UI so that the user gets notified
appropriately).
  • Loading branch information
guyharris committed Aug 2, 2022
1 parent 4d91679 commit 71f32ef
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 1 deletion.
30 changes: 30 additions & 0 deletions editcap.c
Original file line number Diff line number Diff line change
Expand Up @@ -1284,6 +1284,21 @@ main(int argc, char *argv[])

case LONGOPT_CAPTURE_COMMENT:
{
/*
* Make sure this would fit in a pcapng option.
*
* XXX - 65535 is the maximum size for an option in pcapng;
* what if another capture file format supports larger
* comments?
*/
if (strlen(ws_optarg) > 65535) {
/* It doesn't fit. Tell the user and give up. */
cmdarg_err("Capture comment %u is too large to save in a capture file.",
capture_comments->len + 1);
ret = INVALID_OPTION;
goto clean_exit;
}

/* pcapng supports multiple comments, so support them here too.
*/
if (!capture_comments) {
Expand Down Expand Up @@ -1311,6 +1326,21 @@ main(int argc, char *argv[])
goto clean_exit;
}

/*
* Make sure this would fit in a pcapng option.
*
* XXX - 65535 is the maximum size for an option in pcapng;
* what if another capture file format supports larger
* comments?
*/
if (strlen(ws_optarg+string_start_index) > 65535) {
/* It doesn't fit. Tell the user and give up. */
cmdarg_err("A comment for frame %u is too large to save in a capture file.",
frame_number);
ret = INVALID_OPTION;
goto clean_exit;
}

/* Lazily create the table */
if (!frames_user_comments) {
frames_user_comments = g_tree_new_full(framenum_compare, NULL, NULL, g_free);
Expand Down
15 changes: 15 additions & 0 deletions ui/qt/capture_file_properties_dialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "capture_file_properties_dialog.h"
#include <ui_capture_file_properties_dialog.h>

#include "ui/simple_dialog.h"
#include "ui/summary.h"

#include "wsutil/str_util.h"
Expand Down Expand Up @@ -616,6 +617,20 @@ void CaptureFilePropertiesDialog::on_buttonBox_accepted()
if (wtap_dump_can_write(cap_file_.capFile()->linktypes, WTAP_COMMENT_PER_SECTION))
{
gchar *str = qstring_strdup(ui->commentsTextEdit->toPlainText());

/*
* Make sure this would fit in a pcapng option.
*
* XXX - 65535 is the maximum size for an option in pcapng;
* what if another capture file format supports larger
* comments?
*/
if (strlen(str) > 65535) {
/* It doesn't fit. Tell the user and give up. */
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
"That coment is too large to save in a capture file.");
return;
}
cf_update_section_comment(cap_file_.capFile(), str);
emit captureCommentChanged();
fillDetails();
Expand Down
27 changes: 26 additions & 1 deletion ui/qt/packet_list.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include "ui/recent.h"
#include "ui/recent_utils.h"
#include "ui/ws_ui_util.h"
#include "ui/simple_dialog.h"
#include <wsutil/utf8_entities.h>
#include "ui/util.h"

Expand Down Expand Up @@ -1415,6 +1416,18 @@ void PacketList::addPacketComment(QString new_comment)

wtap_block_t pkt_block = cf_get_packet_block(cap_file_, fdata);

/*
* Make sure this would fit in a pcapng option.
*
* XXX - 65535 is the maximum size for an option in pcapng;
* what if another capture file format supports larger
* comments?
*/
if (ba.size() > 65535) {
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
"That coment is too large to save in a capture file.");
return;
}
wtap_block_add_string_option(pkt_block, OPT_COMMENT, ba.data(), ba.size());

cf_set_modified_block(cap_file_, fdata, pkt_block);
Expand All @@ -1441,6 +1454,18 @@ void PacketList::setPacketComment(guint c_number, QString new_comment)
wtap_block_remove_nth_option_instance(pkt_block, OPT_COMMENT, c_number);
} else {
QByteArray ba = new_comment.toLocal8Bit();
/*
* Make sure this would fit in a pcapng option.
*
* XXX - 65535 is the maximum size for an option in pcapng;
* what if another capture file format supports larger
* comments?
*/
if (ba.size() > 65535) {
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
"That coment is too large to save in a capture file.");
return;
}
wtap_block_set_nth_string_option_value(pkt_block, OPT_COMMENT, c_number, ba.data(), ba.size());
}

Expand Down Expand Up @@ -2132,4 +2157,4 @@ void PacketList::resizeAllColumns(bool onlyTimeFormatted)
resizeColumnToContents(col);
}
}
}
}

0 comments on commit 71f32ef

Please sign in to comment.