Verified that the tests failed without the fixes for the linked bugs.
The tests have full statement coverage(*1) for check_follow_fragments
and follow_tcp_tap_listener. For details and Scapy script, see:
https://git.lekensteyn.nl/peter/wireshark-notes/commit/crafted-pkt/badsegments.py?id=4ecf9d858b49e76d8a9c29df01ce1bd523ae6704
(*1) except for `if (data_length <= data_offset) { data_length = 0; }`
Change-Id: I625536df375272cf6c9116231194c39df1217fae
Ping-Bug: 13700
Ping-Bug: 14944
Reviewed-on: https://code.wireshark.org/review/28618
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
It strips off all suffixes, which is not useful behavior; it assumes
that nobody uses "." for any purpose other than separating a file name
from an extension - 1994 called, they want their version of Windows
back (and UN*X called, too...).
For the "Saving XXX" status bar message, just use the entire last
component of the file name.
Change-Id: Ib34fde3e49cd791c7baf333eebb71a8dbd672c19
Reviewed-on: https://code.wireshark.org/review/28638
Reviewed-by: Guy Harris <guy@alum.mit.edu>
Strip off only extensions that correspond to file types we know about;
QFileInfo::baseName() strips off *all* extensions, where "extension" is
"anything preceded by a .", so it turns foo.bar.pcap.gz into foo, not
foo.bar. We don't want that; instead, we strip off only those
extensions that correspond to file types we know how to read, so we'd
strip off .pcap.gz in foo.bar.pcap.gz, and strip off .pcap in
foo.bar.pcap, leaving foo.bar in both cases.
Change-Id: I5385921ad2f0fef815d52e9902fef15735fd9dae
Reviewed-on: https://code.wireshark.org/review/28636
Reviewed-by: Guy Harris <guy@alum.mit.edu>
1. CIP Generic I/O: Add a "Decode As" option for CIP Class 1
2. Combine the 2 CIP conversation filters into one. There would never have
been more than 1 selected. This should be easier for users and less
clutter in the conversation menu.
3. CIP Safety: Name the data field as cipsafety.data instead of
enip.connection_transport_data, to make it like other CIP I/O data.
4. Minor: Rename some more variables/functions from enip to cip, to
reflect which actual protocol these are.
Change-Id: Id895f412e3584a5efcb7e69175a1b2bb3d5e9627
Reviewed-on: https://code.wireshark.org/review/28610
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
1. Use proto_tree_add_bitmask for the flags example
2. GLib download link was dead
3. Remove old frontend information. I can't find any download for
hethereal, and Packetyzer is so old that it's not useful for any current
developers.
Change-Id: Ifa0a7363fccb95fb2ef315d84fbbcf7414ae6a6d
Reviewed-on: https://code.wireshark.org/review/28632
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
When switching from ASCII to other modes (such as Hex), the previous
text to packet number mapping was not cleared. This resulted in
using the wrong packet number when hovering over the packet data.
Change-Id: I29ba1786925490c33fc9181373a31d51f5091642
Reviewed-on: https://code.wireshark.org/review/28614
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
In the Qt Follow TCP Stream dialog with the ASCII mode, sometimes
selecting the first few bytes would wrongly select a packet with a
higher frame number.
This happens because Qt iterates through the list of payloads, then
stores appends the payload data and maps the new cursor position to the
packet number. If the payload data was empty, then it would overwrite
previous cursor positions.
To fix this, do not add records for empty TCP payloads.
Bug: 14898
Change-Id: I598d73899b56eac3d2a022f108bf097bdd363b5c
Reviewed-on: https://code.wireshark.org/review/28613
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
When two segments overlap each other, previously the second segment
would wrongly be appended to the first one while it should only append a
subset from the end of the second segment. (It is assumed that the very
first segment is received in time such that an extension on both the
left and right side of the previous stream is not possible.)
Make sure that "frag_follow_record->data" uses a subset (starting at the
end of the previous stream) instead of the full tvb contents. While at
it, add some documentation and restructure the logic to avoid code
duplication and unnecessary memory allocations.
(From bug 9882:) Tested with hao123-com_packet-injection-filtered.pcap,
it now ignores the new overlapping data. Likewise for overlap-2.pcap.
Tested with retransmission_with_additional_payload.pcapng (bug 13700).
Unfortunately, there is no extra expert info to warn about the
non-matching overlapping segment data, but that is a separate issue.
Bug: 13700
Change-Id: I74a941199d75b23b5d297e4dd534680ae610627d
Reviewed-on: https://code.wireshark.org/review/28597
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Restore the direction for the first check_follow_fragments call to match
the situation before commit 57acc227f0 (which broke other things, so its
logic was reimplemented in commit v2.3.0rc0-1449-g66fa31415f ("tcp: Fix
Follow TCP tap data and when its tapped.")).
Ensure that the ACK value is checked before processing the sequence
number and payload for the current flow.
Bug: 14944
Change-Id: If8947d7732683a4943f405eb72b1a8526a35a6dc
Fixes: v2.1.0rc0-1339-g57acc227f0 ("KISS the Follow TCP functionality.")
Reviewed-on: https://code.wireshark.org/review/28612
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
To improve readability, do not repeat "follow_record" a dozen times.
No functional change.
Change-Id: I854434974a94d69d4591ad1bc3acf911073b0923
Reviewed-on: https://code.wireshark.org/review/28596
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Reverse the payload chunks list to achieve a running time of O(n) rather
than O(n²) for insertion of all chunks. Executing a RelWithDebInfo+ASAN
build with `tshark -r chargen-session.pcapng.gz -qz follow,tcp,hex,0`
previously took 11m5s to complete, but now finishes in 16 seconds.
Tested using a capture file with 152k TCP packets (from bug 11777).
Backport note: must update ui/gtk/follow_stream.c too.
Change-Id: Icf70d45f33d4399e53209fb6199d3809608c8d99
Reviewed-on: https://code.wireshark.org/review/28595
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
For example, if the file is foo.pcap, make the default name for a saved
PDF of some graph be foo.pdf, as it was prior to 2.6, not foo.pcap.pdf.
Change-Id: Ide99c9c7fa1f3d16f829e731f968a209fbb52b8d
Reviewed-on: https://code.wireshark.org/review/28624
Reviewed-by: Guy Harris <guy@alum.mit.edu>
While we're at it, sort some method declarations and definitions, to
group the top-level summary/details/bytes yes/no options together, with
two groups of suboptions for summary and details below.
Bug: 14945
Change-Id: Id06dd64e44b18b13e2131482edef46aee3efbd63
Reviewed-on: https://code.wireshark.org/review/28620
Reviewed-by: Guy Harris <guy@alum.mit.edu>
The EAPOL-ready field has not been part of the specification since
before this dissector has been created.
Change-Id: I7bd25a44ad3ee61e7a2a3b105f7bbffb1e28b31f
Reviewed-on: https://code.wireshark.org/review/28602
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Some flags in the connectionless PDU header are "reserved for
implementation", which presumably means an implementation can set them
to 0 or 1 and use it to send information to a compatible implementation;
others are "reserved for future use" and "must be set to 0".
Don't test the "reserved for implementation" flags in the heuristic, and
show them as "Reserved for implementation" and show the others as
"Reserved for future use (MBZ)".
Bug: 14942
Change-Id: Iff40f155e057301096fec1dbb68f71d041508ff1
Reviewed-on: https://code.wireshark.org/review/28598
Reviewed-by: Guy Harris <guy@alum.mit.edu>
For filePath() and fileName(), just return a null string if we can't
convert from the native encoding to UTF-8 - those aren't used for
displaying, those are used for setting the main window's file name and
for generating names of files to save based on the capture file name.
Have fileDisplayName() just return the display name, without
"[closing]"/"[closed]" decoration or a special case for no file being
open (just return a null string if there's no file open), and have
fileTitle() return the decorated display name.
Change-Id: I244f318d5444dcf58527e5d38c4d073c28b73810
Reviewed-on: https://code.wireshark.org/review/28594
Reviewed-by: Guy Harris <guy@alum.mit.edu>
That more closely matches the name of the file.h routine that it uses.
Change-Id: Ia206fb8331f4f3ad8035da9f6137ad2428d53a49
Reviewed-on: https://code.wireshark.org/review/28589
Reviewed-by: Guy Harris <guy@alum.mit.edu>
That's needed to support the "proxy icon", so it can be dragged.
Change-Id: I1ad209cd43a2a6df9c52d076f6513780b0ac51be
Reviewed-on: https://code.wireshark.org/review/28587
Reviewed-by: Guy Harris <guy@alum.mit.edu>
It now does the heavy lifting, so MainWindow::setTitlebarForCaptureFile()
doesn't have to duplicate it.
Change-Id: I97ded85306e625b2c67c3fde62a636ec6818a6f5
Reviewed-on: https://code.wireshark.org/review/28586
Reviewed-by: Guy Harris <guy@alum.mit.edu>
Don't use CaptureFile.fileTitle() if you're constructing a pathname; use
it only if you're constructing a window title.
Change-Id: I40f225ddb07be2f7dc3ae03108dae816846f20c7
Reviewed-on: https://code.wireshark.org/review/28582
Reviewed-by: Guy Harris <guy@alum.mit.edu>
For dialogs and auxiliary windows, if we have a live capture that hasn't
yet been saved to a permanent location, there's no good reason to show
the temporary file name in the title bar, as:
it's a random string that doesn't indicate where the capture was done
and that could confuse people (see, for example, the confusion in bug
14929, in which somebody referred to the "Follow TCP Stream" window as
the ".pcap dialog" because its title had ".pcap" at the end, due to
the capture file being a temporary file and its name showing up in the
title bar of that window);
it differs from what the main window title bar shows.
While we're at it, don't assume that the file name in the capture_file
structure is a UTF-8 string - some UN*Xes might not use UTF-8 for file
names.
Change-Id: I0d3dfd5d7f896ea37533daf7089b688710dbabf0
Reviewed-on: https://code.wireshark.org/review/28581
Reviewed-by: Guy Harris <guy@alum.mit.edu>
On Ubuntu trusty, Asciidoctor 1.4 is installed. This does not satisfy
the minimum version requirement (1.5) and should not be used even if the
binary is available.
Change-Id: Iaffd55a5bcb26510b4b59f209768a61c3116d32f
Fixes: v2.5.1rc0-76-g94a0f7c641 ("Switch from AsciiDoc to Asciidoctor.")
Reviewed-on: https://code.wireshark.org/review/28576
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Addressing code review comments from Peter Wu.
Bug: 14935
Change-Id: I5e2dbad1ab42c3f958b29092df31d3636d04812c
Reviewed-on: https://code.wireshark.org/review/28569
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
As "cf_read" and "rescan_packets" can end up calling back to the GUI
code, that could destroy "cf->epan" which could result in use-after-free
crashes. While I can find most issues with ASAN, it would be even
better to detect the destructive action in "cf_close".
Change-Id: I72700a60c6786d153c2aaa8478bfdfb16a01dcda
Ping-Bug: 10870
Reviewed-on: https://code.wireshark.org/review/28542
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Closing a capture file while it is being loaded will result in a crash.
As a workaround, disallow closing the capture file. The requested action
(e.g. MainWindow::openCaptureFile) will be silently ignored.
While at it, protect process_specified_records (called when saving
files) similarly to cf_read and fix a crash that occurs when a capture
from the Capture Dialog is started while a file is being loaded:
file.c:360:cf_close: assertion failed: (cf->state != FILE_READ_IN_PROGRESS)
Bug: 10870 # moving rapidly between large files in a file set
Bug: 13594 # start capture while loading/saving file
Bug: 14351 # open another file while loading file
Change-Id: I6ce8f3163c3fa4869f0299e49909a32594326ce4
Reviewed-on: https://code.wireshark.org/review/28541
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
During live captures, "cf->state==FILE_READ_IN_PROGRESS" holds and as
such setting "cf->redissection_queued" from "cf_filter_packets" will
prevent the packet list from being updated (no new packets are added and
display filter changes are not applied).
Fix this by not checking "cf->state" and instead perform an explicit
check to detect the "update_progress_dlg" issue (see original commit).
As "cf->read_lock" is implied by "cf->redissecting", remove that check
as well (see "rescan_packets").
Print a warning instead of aborting in "cf_read" since I am not sure if
that condition is currently prevented by its callers.
Bug: 14918
Change-Id: Ieb7d1ae3cbeef18f17c850ae3778822ee625dc68
Fixes: v2.9.0rc0-1110-g8e07b778f6 ("file: do not perform recursive redissections to avoid crashes")
Reviewed-on: https://code.wireshark.org/review/28538
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Use h265_profile_idc_values as the value_string for
h265.general_profile_idc. Get rid of a duplicate value - 2 is used for
both "Main 10" and "Main 10 Still" profiles, a *separate* part of the
packet indicates whether it's the still picture version or not.
"#if 0" out h265_level_bitrate_values - it's not clear where it should
be used.
Initialize two-dimensional arrays with { { 0 } }, i.e. an array of
arrays, with the first element of the first array explicitly initialized
to zero, and all the other elements of all the other arrays implicitly
initialized to zero.
Change-Id: Ia2ddc28528dcc49fa7a69685b7e5d08d2cd6b4e7
Reviewed-on: https://code.wireshark.org/review/28574
Reviewed-by: Guy Harris <guy@alum.mit.edu>
For better type safety
Change-Id: Ida7b98af8c44a52ddac2c4ab0702db2519a0c4af
Reviewed-on: https://code.wireshark.org/review/28570
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
Peter Wu