Commit Graph

121 Commits

Author SHA1 Message Date
Peter Wu c5b2c1e8f4 reassemble: remove special treatment for truncated data
Do not try to recover from truncated tvbs for fragment_add_seq-like
functions:

 - If it is the first block and the dissector requested frag_data_len
   number of bytes, we should not lie and pretend that we are fully
   reassembled.
 - For other blocks, returning NULL as no reassembly was possible makes
   sense. But other fragments in the list should not be cleared as there
   may be partial fragments which were returned before.

It seems that this special behavior was introduced in
b2c11b5e13 (freeing fragments and
returning NULL as an optimization when fragments are deemed not needed
anymore) and faeb2c2ee1 (for returning
fd_head for the first fragment, "so the first fragment gets dissected as
fragmented packet").

Now in theory unused fragments could stick around, but that also
possible with the normal fragment_add functions.

Bug: 11799
Change-Id: I20829c54e1b2eee25a91fe4de51b19b1458c7789
Reviewed-on: https://code.wireshark.org/review/14082
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2016-04-03 16:22:46 +00:00
João Valverde e4c059f67f Add free_address_wmem(), fix warnings [-Wcast-qual]
Try to improve address API and also fix some constness warnings
by not overloading the 'data' pointer to store malloc'ed buffers
(use private pointer for that instead).

Second try, now passing test suite.

Change-Id: Idc101cd866b6d4f13500c9d59da5c7a38847fb7f
Reviewed-on: https://code.wireshark.org/review/13946
Petri-Dish: João Valverde <j@v6e.pt>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: João Valverde <j@v6e.pt>
2016-02-26 23:09:43 +00:00
Peter Wu c2f85b6925 Extend reassembly documentation
Documentation changes only (comments and docbook).

Update WSDG with the fragment_add_seq_check API that was introduced in
Wireshark 1.10.

Fix typos and clarify the many functions we have for adding reassembling
fragments.

Change-Id: I38715a8f58e9cf1fe3e34ee4b1a4ae339630282b
Reviewed-on: https://code.wireshark.org/review/14066
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2016-02-24 06:18:47 +00:00
Guy Harris 8bb1e58cbc Suggest pushing a decision lower in the code.
That leaves less room for getting it wrong.

Change-Id: Iea003fc102ccd14db2924b70fc685033ca34f291
Reviewed-on: https://code.wireshark.org/review/13863
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2016-02-10 02:00:11 +00:00
João Valverde ef929dc8eb Revert "Add free_address_wmem() and other extensions to address API"
This reverts commit 13ec77a9fc.

This commit introduces a segmentation fault for Lua code (uncovered by the test suite).

Change-Id: Ibc273d1915cda9632697b9f138f0ae104d3fb65e
Reviewed-on: https://code.wireshark.org/review/13813
Reviewed-by: João Valverde <j@v6e.pt>
2016-02-08 00:44:22 +00:00
João Valverde 13ec77a9fc Add free_address_wmem() and other extensions to address API
Try to improve 'address' API (to be easier/safer) and also avoid
some constness warnings by not overloading the 'data' pointer to
store malloc'ed buffers (use private pointer for that instead).

Change-Id: I7456516b12c67620ceadac447907c12f5905bd49
Reviewed-on: https://code.wireshark.org/review/13463
Petri-Dish: João Valverde <j@v6e.pt>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: João Valverde <j@v6e.pt>
2016-02-07 23:22:30 +00:00
Guy Harris bc5a0374bf Add the packet number to the packet_info structure, and use it.
That removes most of the uses of the frame number field in the
frame_data structure.

Change-Id: Ie22e4533e87f8360d7c0a61ca6ffb796cc233f22
Reviewed-on: https://code.wireshark.org/review/13509
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2016-01-24 03:41:28 +00:00
João Valverde 8fb1f8e552 reassemble: Add key destroy function to g_hash_table
Fixes memleak in reassemble.c

480 bytes in 60 blocks are definitely lost in loss record 3,010 of 3,059
   at 0x4C28C10: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0xADA3328: g_malloc (in /usr/lib/libglib-2.0.so.0.4600.1)
   by 0xADBA512: g_slice_alloc (in /usr/lib/libglib-2.0.so.0.4600.1)
   by 0x6575C7D: fragment_reassembled (reassemble.c:804)
   by 0x6577785: fragment_add_seq_check_work (reassemble.c:2027)
   by 0x6577880: fragment_add_seq_next (reassemble.c:2068)
   by 0x6E614E6: dissect_sccp_message (packet-sccp.c:2875)
   by 0x6E63641: dissect_sccp (packet-sccp.c:3401)
   by 0x6546CF7: call_dissector_through_handle (packet.c:620)
   by 0x6546EA1: call_dissector_work (packet.c:706)
   by 0x6547A04: dissector_try_uint_new (packet.c:1163)
   by 0x6547A65: dissector_try_uint (packet.c:1189)

Change-Id: I0117b48e1e5d5688c49f264f24387dd6de1d6e08
Reviewed-on: https://code.wireshark.org/review/11541
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-11-06 04:38:07 +00:00
Guy Harris ff9f9fc431 Don't include "file.h" if you don't need it.
It ends up dragging in libwireshark headers, which programs not linking
with libwireshark shouldn't do.  In particular, including
<epan/address.h> causes some functions that refer to libwireshark
functions to be defined if the compiler doesn't handle "static inline"
the way GCC does, and you end up requiring libwireshark even though you
shouldn't require it.

Move plurality() to wsutil/str_util.h, so that non-libwireshark code can
get it without include epan/packet.h.  Fix includes as necessary.

Change-Id: Ie4819719da4c2b349f61445112aa419e99b977d3
Reviewed-on: https://code.wireshark.org/review/11545
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2015-11-04 08:46:30 +00:00
Gerald Combs 86fe2be4dc Use address functions instead of ADDRESS macros in asn1 and epan
Replace CMP_ADDRESS, COPY_ADDRESS, et al with their lower-case
equivalents in the asn1 and epan directories.

Change-Id: I4043b0931d4353d60cffbd829e30269eb8d08cf4
Reviewed-on: https://code.wireshark.org/review/11200
Petri-Dish: Michal Labedzki <michal.labedzki@tieto.com>
Petri-Dish: Alexis La Goutte <alexis.lagoutte@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
2015-10-22 11:45:56 +00:00
Evan Huus 9b5b7c1a90 Remove deprecated tvb_length calls
Trust that the files in epan/ immediately (not dissectors) know what they're
doing so just blindly convert them to captured length.

Change-Id: I872f7d58b2e15ae82c75fd56f4873996fbc97be7
Reviewed-on: https://code.wireshark.org/review/9083
Reviewed-by: Evan Huus <eapache@gmail.com>
2015-06-24 00:31:25 +00:00
Evan Huus c35f2ccb44 reassembly: address a 15-year old XXX comment
Question: "what if we didn't capture the entire fragment due to a too-short
          snapshot length?"
Answer: An assertion fails and we leak a bunch of memory.

Don't do that.

Bug: 11129
Change-Id: I0adfb217f0e66ae8f5f6255a4caf7ff940826b59
Reviewed-on: https://code.wireshark.org/review/8128
Petri-Dish: Evan Huus <eapache@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Evan Huus <eapache@gmail.com>
2015-04-21 01:05:17 +00:00
AndersBroman 98b7f21370 [Reassembly] Fix a reassembly case where the two fragments are in the same
frame
but in different SCTP DATA chunks, whitout the patch the message is
reassembled in both chunks leading to duplicated upper layer PDU:s in the
frame.

Change-Id: Ie31142c38c728018178947544b571622447d8e8f
Reviewed-on: https://code.wireshark.org/review/7716
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2015-03-18 05:32:06 +00:00
Guy Harris 44e7ce54ff Remove some apparently-unnecessary includes of emem.h.
Change-Id: Ib7d1b587b439ff21ec6b7f1756ce6ccf25b66f80
Reviewed-on: https://code.wireshark.org/review/6635
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2015-01-18 20:19:05 +00:00
Evan Huus 91d1c3c5ec reassembly: fix when fragment cut off by snapshot
Also add an assertion to tvb_generic_clone_offset_len so that it throws an
error *before* allocating memory, as otherwise that memory is leaked.

Bug: 10474
Change-Id: I5036cefac16841914a59670c64979cf599bf7969
Reviewed-on: https://code.wireshark.org/review/4234
Petri-Dish: Evan Huus <eapache@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Evan Huus <eapache@gmail.com>
2014-09-25 12:41:36 +00:00
Bill Meier a379ac74d2 Fix spelling: cant-->can't, wont-->won't, etc
Change-Id: Ia6c3e7a25615bf8e052c3bacf096d76df775c9c2
Reviewed-on: https://code.wireshark.org/review/4126
Reviewed-by: Bill Meier <wmeier@newsguy.com>
2014-09-16 15:31:58 +00:00
Alexis La Goutte 296591399f Remove all $Id$ from top of file
(Using sed : sed -i '/^ \* \$Id\$/,+1 d')

Fix manually some typo (in export_object_dicom.c and crc16-plain.c)

Change-Id: I4c1ae68d1c4afeace8cb195b53c715cf9e1227a8
Reviewed-on: https://code.wireshark.org/review/497
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2014-03-04 14:27:33 +00:00
Bill Meier 11b5c15fdb Remove trailing whitespace
Change-Id: I8116f63ff88687c8db3fd6e8e23b22ab2f759af0
Reviewed-on: https://code.wireshark.org/review/385
Reviewed-by: Bill Meier <wmeier@newsguy.com>
Tested-by: Bill Meier <wmeier@newsguy.com>
2014-02-25 20:46:49 +00:00
Jakub Zawadzki ae59b09443 Add missing includes in order to remove exceptions.h from proto.h (next commit).
svn path=/trunk/; revision=53230
2013-11-10 15:59:37 +00:00
Michael Mann 21ddc7ae4e Revert r53118 as it breaks reassemble_test.
svn path=/trunk/; revision=53130
2013-11-07 04:50:53 +00:00
Michael Mann c59fab0352 Correctly report segments marked with REASSEMBLE_FLAGS_NO_FRAG_NUMBER. Bug 9304 (https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9304)
Instead of incrementing the offset for each new segment by one we add the length of the segment so that each segment is correctly shown in the segment list.

It proves to be very useful to find which packet (segment) is causing an application dissector to go wrong.

From Matthieu Patou

svn path=/trunk/; revision=53118
2013-11-06 21:59:03 +00:00
Evan Huus 54c558fdfc Reorganize handling of incomplete fragments slightly. The logic hasn't changed
much, but I think this way's a little clearer, and it made it much easier to
figure out where the memory leak was.

Fixes the leaks from https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9243

svn path=/trunk/; revision=52448
2013-10-08 15:05:45 +00:00
Michael Mann 12dd69e0b6 Remove check_col.
svn path=/trunk/; revision=52316
2013-10-01 15:14:59 +00:00
Jörg Mayer 029e258225 Explicitly include emem.h where it is needed
svn path=/trunk/; revision=51997
2013-09-13 08:16:50 +00:00
Evan Huus 9fd46c37a8 - no need for a doubly-linked list of TVBs, single is simpler
- support merging chains in tvb_add_to_chain
- when we have an old reassembled TVB, just merge the chains rather than
  freeing it (we may still need it as it may already be a data source)
- modelines

Fixes https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9027

#BACKPORT, but it's gonna be messy...

svn path=/trunk/; revision=51825
2013-09-07 18:20:52 +00:00
Jakub Zawadzki a18172147f Remove fragment_data, add fragment_head, fragment_item - for now alias it to the same structure.
This is begin of work to split fragment head and fragments items.

svn path=/trunk/; revision=50708
2013-07-17 21:12:24 +00:00
Guy Harris b68e6dcc42 A bit more comment updating.
svn path=/trunk/; revision=50590
2013-07-14 20:26:56 +00:00
Guy Harris cc4197b1fe Fix a comment.
svn path=/trunk/; revision=50588
2013-07-14 19:07:09 +00:00
Guy Harris a1ca4be5ef Cast the result of g_malloc(), to avoid warnings about trying to compile
this code as C++.

Make pointers to raw packet data pointers to guint8, not pointers to
char, as they're octets, not characters.

svn path=/trunk/; revision=50583
2013-07-14 17:16:56 +00:00
Jakub Zawadzki c766e78cfa Rewrite reassemble API to use TVBs instead of raw data.
(it seems to be working for TCP ^^)

svn path=/trunk/; revision=50580
2013-07-14 14:42:05 +00:00
Anders Broman ee109cc7b2 From Anthony Coddington:
show_fragment_seq_tree does not add reassembled data tree item

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8766

svn path=/trunk/; revision=49751
2013-06-04 04:23:20 +00:00
Alexis La Goutte ea63ba2c8f Fix some Dead Store (Dead initialization) Warning found by Clang Fix also indent (Use tabs)
svn path=/trunk/; revision=48983
2013-04-22 21:25:12 +00:00
Evan Huus 513698ca76 Fix typo - we want to check if the pointer is NULL, not if the value
pointed to is NULL.

Part of https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8599

svn path=/trunk/; revision=48943
2013-04-20 21:09:41 +00:00
Guy Harris c2087da0ab When we throw a reassembly error, remember the error, so that, if we
revisit this reassembly (in a multi-pass program such as Wireshark, or
TShark with -2), we'll throw the same error.

In fragment_set_tot_len(), allow the length to be set to a value that's
before the offset of existing fragments; we'll catch that later when the
reassembly completes.  This lets us handle some problems with DTLS less
confusingly.

When adding frames to an already-completed reassembly, check for
fragments that overlap existing fragments or go past the end of the
reassembly, and report errors.

When completing a reassembly, make the buffer for the reassembled data
big enough to contain the specified data length for the reassembly, even
if that's less than the offset + length of the last fragment.  Flag all
fragments that go past that length as "too long", and only copy out what
part of them fits, if any.  That lets us flag the correct fragment or
fragments as being "too long".

When adding fragments, do some additional checks, even if we're not
doing the first pass through the packets, so errors that show up in the
first pass also show up on subsequent passes.

svn path=/trunk/; revision=48909
2013-04-18 02:31:45 +00:00
Guy Harris da76e5a997 Include a message with all ReassemblyError exceptions; they're expected
to have a message indicating the precise error.

svn path=/trunk/; revision=48554
2013-03-25 22:36:29 +00:00
Evan Huus 0314bd1c1b From Roland Knall via https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8502
Allow reassembly of sequences when the sequence numbers do not count from 0.

svn path=/trunk/; revision=48548
2013-03-25 12:53:26 +00:00
Anders Broman 5abba34e9c make local functions static.
svn path=/trunk/; revision=48523
2013-03-24 14:03:12 +00:00
Guy Harris eee846b9c6 Make reassembly_table_destroy() clear the function pointers and empty
and destroy the reassembled table.

svn path=/trunk/; revision=48492
2013-03-23 02:40:37 +00:00
Guy Harris a2414d8909 Don't wire into the reassembly code the notion that reassemblies should
be done on flows from one address to another; reassembly for protocols
running atop TCP should be done on flows from one TCP endpoint to
another.

We do this by:

	adding "reassembly table" as a data structure;

	associating hash tables for both in-progress reassemblies and
	completed reassemblies with that data structure (currently, not
	all reassemblies use the latter; they might keep completed
	reassemblies in the first table);

	having functions to create and destroy keys in that table;

	offering standard routines for doing address-based and
	address-and-port-based flow processing, so that dissectors not
	needing their own specialized flow processing can just use them.

This fixes some mis-reassemblies of NIS YPSERV YPALL responses (where
the second YPALL response is processed as if it were a continuation of
a previous response between different endpoints, even though said
response is already reassembled), and also allows the DCE RPC-specific
stuff to be moved out of epan/reassembly.c into the DCE RPC dissector.

svn path=/trunk/; revision=48491
2013-03-22 23:59:54 +00:00
Anders Broman 94d102adb4 From beroset:
remove C++ incompatibilities 
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8416

svn path=/trunk/; revision=48318
2013-03-15 16:36:31 +00:00
Evan Huus f6156140c3 Followup to r48011, answering my own XXX comment:
No, ReportedBoundsError is not the right thing to throw, ReassemblyError is.
That's why I added it in the first place!

svn path=/trunk/; revision=48123
2013-03-06 00:39:35 +00:00
Evan Huus b20db86a6f Define a new exception for reassembly errors, and throw it in several cases
instead of using DISSECTOR_ASSERT. When a dissector passes bad data to the
reassembly machine, that isn't necessarily the dissector's fault - the data may
come straight from the packet, and the dissector may not have enough information
to know it's bad without telling the reassembly machine in the first place.

Also fix a bug in the reassembly machine. If it were given a fragment and all of
the following conditions were met:
- the other associated fragments were already marked as done (reassembled)
- the fragment went beyond the end of the conceptual reassembled buffer
- the dissector had not set the PARTIAL_REASSEMBLY flag
then the reassembly machine would incorrectly think there was an overlap and
run past the end of the already-reassembled buffer.

Should fix the rest of
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=8380

#BACKPORT
This is probably too big and intrusive to backport directly, and parts of it
will need adapting anyways since reassemble.c has changed. But the bug exists
and crashes in 1.6 and 1.8, so we'll have to do something.


svn path=/trunk/; revision=48011
2013-03-02 16:39:56 +00:00
Martin Kaiser d3b504f331 From Evan:
sanity checks before setting a packet's total length in
fragment_set_tot_len()

(from me: check if fragments exist for the given id)

hopefully, this fixes #8111 and #8163 without causing troubles for other
protocols that use fragmentation and reassembly

svn path=/trunk/; revision=46999
2013-01-08 22:18:52 +00:00
Evan Huus 347a8a2115 Add full modelines and minor whitespace fixes.
svn path=/trunk/; revision=46812
2012-12-27 18:58:08 +00:00
Jeff Morriss 3729335973 We always HAVE_CONFIG_H so don't bother checking whether we have it or not.
svn path=/trunk/; revision=45016
2012-09-20 01:48:30 +00:00
Jeff Morriss aa5d9d78dd From Robert Bullen via https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7683 :
The reassembled fragments tree in the Packet Details view is awesome, but it
lacks one thing: a field that exposes the reassembled data.

tcp.data already exists for exposing a single TCP segment's payload as a byte
array. It would be handy to have something similar for a single application
layer PDU when TCP segment reassembly is involved. I propose
tcp.reassembled.data, named and placed after the already existing field
tcp.reassembled.length.

My primary use case for this feature is outputting tcp.reassembled.data with
tshark for further processing with a script.

The attached patch implements this very feature. Because the reassembled
fragment tree code is general purpose, i.e. not specific to just TCP, any
dissector that relies upon it can add a similar field very cheaply. In that
vein I've also implemented ip.reassembled.data and ipv6.reassembled.data, which
expose reassembled fragment data as a single byte stream for IPv4 and IPv6,
respectively. All other protocols that use the reassembly code have been left
alone, other than inserting NULL into their initializer lists for the newly
introduced struct field reassemble.h:fragment_items.hf_reassembled_data.

svn path=/trunk/; revision=44802
2012-09-07 02:09:59 +00:00
Bill Meier 38b39b6b77 Add 'fragment_table_destroy()'
svn path=/trunk/; revision=44459
2012-08-12 20:53:17 +00:00
Bill Meier 103ddcbfd5 Use g_slice allocation for 'reassembled_table' keys (instead of se allocation).
Not strictly required, but IMO a bit cleaner (if maybe a bit less efficient).

svn path=/trunk/; revision=44382
2012-08-09 14:55:13 +00:00
Bill Meier 3c7d32017f From Jakub Zawadzki: Fix for memory leak in reassemble.c
Bug #4141

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=4141#c10

svn path=/trunk/; revision=44371
2012-08-09 02:06:53 +00:00
Jakub Zawadzki bf81b42e1e Update Free Software Foundation address.
(COPYING will be updated in next commit)

svn path=/trunk/; revision=43536
2012-06-28 22:56:06 +00:00