forked from osmocom/wireshark
![Peter Wu](/assets/img/avatar_default.png)
Removed all guards for HAVE_LIBGCRYPT, change autotools and CMake to error out if it is not available. Update release notes, developer documentation and README with the new status. Clarify relation with GnuTLS in macosx-setup.sh. Install Libgcrypt via brew script. Motivation for this change is that many dissectors depend on Libgcrypt and having it optional increases the maintenance burden (there have been several compile issues in the past due to the optional status). Furthermore, wsutil has crypto code that can be replaced by Libgcrypt. Change-Id: Idf0021b8c4cd5db70b8766f7dcc2a8b3acbf042f Link: https://www.wireshark.org/lists/wireshark-dev/201702/msg00011.html Reviewed-on: https://code.wireshark.org/review/20030 Petri-Dish: Peter Wu <peter@lekensteyn.nl> Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org> Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com> Reviewed-by: Peter Wu <peter@lekensteyn.nl>
729 lines
30 KiB
Text
729 lines
30 KiB
Text
Tabsize: 4
|
|
|
|
This file is a HOWTO for Wireshark developers. It describes general development
|
|
and coding practices for contributing to Wireshark no matter which part of
|
|
Wireshark you want to work on.
|
|
|
|
To learn how to write a dissector, read this first, then read the file
|
|
README.dissector.
|
|
|
|
This file is compiled to give in depth information on Wireshark.
|
|
It is by no means all inclusive and complete. Please feel free to send
|
|
remarks and patches to the developer mailing list.
|
|
|
|
0. Prerequisites.
|
|
|
|
Before starting to develop a new dissector, a "running" Wireshark build
|
|
environment is required - there's no such thing as a standalone "dissector
|
|
build toolkit".
|
|
|
|
How to setup such an environment is platform dependent; detailed information
|
|
about these steps can be found in the "Developer's Guide" (available from:
|
|
https://www.wireshark.org) and in the INSTALL and README files of the sources
|
|
root dir.
|
|
|
|
0.1. General README files.
|
|
|
|
You'll find additional information in the following README files:
|
|
|
|
- README.capture - the capture engine internals
|
|
- README.design - Wireshark software design - incomplete
|
|
- README.developer - this file
|
|
- README.dissector - How to dissect a packet
|
|
- README.display_filter - Display Filter Engine
|
|
- README.idl2wrs - CORBA IDL converter
|
|
- README.packaging - how to distribute a software package containing WS
|
|
- README.regression - regression testing of WS and TS
|
|
- README.stats_tree - a tree statistics counting specific packets
|
|
- README.tapping - "tap" a dissector to get protocol specific events
|
|
- README.xml-output - how to work with the PDML exported output
|
|
- wiretap/README.developer - how to add additional capture file types to
|
|
Wiretap
|
|
|
|
0.2. Dissector related README files.
|
|
|
|
You'll find additional dissector related information in the file
|
|
README.dissector as well as the following README files:
|
|
|
|
- README.heuristic - what are heuristic dissectors and how to write them
|
|
- README.plugins - how to "pluginize" a dissector
|
|
- README.request_response_tracking - how to track req./resp. times and such
|
|
- README.wmem - how to obtain "memory leak free" memory
|
|
|
|
0.3 Contributors
|
|
|
|
James Coe <jammer[AT]cin.net>
|
|
Gilbert Ramirez <gram[AT]alumni.rice.edu>
|
|
Jeff Foster <jfoste[AT]woodward.com>
|
|
Olivier Abad <oabad[AT]cybercable.fr>
|
|
Laurent Deniel <laurent.deniel[AT]free.fr>
|
|
Gerald Combs <gerald[AT]wireshark.org>
|
|
Guy Harris <guy[AT]alum.mit.edu>
|
|
Ulf Lamping <ulf.lamping[AT]web.de>
|
|
|
|
1. Portability.
|
|
|
|
Wireshark runs on many platforms, and can be compiled with a number of
|
|
different compilers; here are some rules for writing code that will work
|
|
on multiple platforms.
|
|
|
|
In general, not all C99 features can be used since some C compilers used to
|
|
compile Wireshark, such as Microsoft's C compiler, don't support all C99
|
|
features. The C99 features that can be used are:
|
|
|
|
- flexible array members
|
|
- compound literals
|
|
- designated initializers
|
|
- "//" comments
|
|
- mixed declarations and code
|
|
- new block scopes for selection and iteration statements (that is, declaring
|
|
the type in a for-loop like: for (int i = 0; i < n; i++) ;)
|
|
- macros with a variable number of arguments (variadic macros)
|
|
- trailing comma in enum declarations
|
|
- inline functions (guaranteed only by use of glib.h)
|
|
|
|
Don't initialize variables in their declaration with non-constant
|
|
values. Not all compilers support this. E.g. don't use
|
|
guint32 i = somearray[2];
|
|
use
|
|
guint32 i;
|
|
i = somearray[2];
|
|
instead.
|
|
|
|
Don't use zero-length arrays as structure members, use flexible array members
|
|
instead.
|
|
|
|
Don't use anonymous unions; not all compilers support them.
|
|
Example:
|
|
|
|
typedef struct foo {
|
|
guint32 foo;
|
|
union {
|
|
guint32 foo_l;
|
|
guint16 foo_s;
|
|
} u; /* have a name here */
|
|
} foo_t;
|
|
|
|
Don't use "uchar", "u_char", "ushort", "u_short", "uint", "u_int",
|
|
"ulong", "u_long" or "boolean"; they aren't defined on all platforms.
|
|
If you want an 8-bit unsigned quantity, use "guint8"; if you want an
|
|
8-bit character value with the 8th bit not interpreted as a sign bit,
|
|
use "guchar"; if you want a 16-bit unsigned quantity, use "guint16";
|
|
if you want a 32-bit unsigned quantity, use "guint32"; and if you want
|
|
an "int-sized" unsigned quantity, use "guint"; if you want a boolean,
|
|
use "gboolean". Use "%d", "%u", "%x", and "%o" to print those types;
|
|
don't use "%ld", "%lu", "%lx", or "%lo", as longs are 64 bits long on
|
|
many platforms, but "guint32" is 32 bits long.
|
|
|
|
Don't use "long" to mean "signed 32-bit integer", and don't use
|
|
"unsigned long" to mean "unsigned 32-bit integer"; "long"s are 64 bits
|
|
long on many platforms. Use "gint32" for signed 32-bit integers and use
|
|
"guint32" for unsigned 32-bit integers.
|
|
|
|
Don't use "long" to mean "signed 64-bit integer" and don't use "unsigned
|
|
long" to mean "unsigned 64-bit integer"; "long"s are 32 bits long on
|
|
many other platforms. Don't use "long long" or "unsigned long long",
|
|
either, as not all platforms support them; use "gint64" or "guint64",
|
|
which will be defined as the appropriate types for 64-bit signed and
|
|
unsigned integers.
|
|
|
|
On LLP64 data model systems (notably 64-bit Windows), "int" and "long"
|
|
are 32 bits while "size_t" and "ptrdiff_t" are 64 bits. This means that
|
|
the following will generate a compiler warning:
|
|
|
|
int i;
|
|
i = strlen("hello, sailor"); /* Compiler warning */
|
|
|
|
Normally, you'd just make "i" a size_t. However, many GLib and Wireshark
|
|
functions won't accept a size_t on LLP64:
|
|
|
|
size_t i;
|
|
char greeting[] = "hello, sailor";
|
|
guint byte_after_greet;
|
|
|
|
i = strlen(greeting);
|
|
byte_after_greet = tvb_get_guint8(tvb, i); /* Compiler warning */
|
|
|
|
Try to use the appropriate data type when you can. When you can't, you
|
|
will have to cast to a compatible data type, e.g.
|
|
|
|
size_t i;
|
|
char greeting[] = "hello, sailor";
|
|
guint byte_after_greet;
|
|
|
|
i = strlen(greeting);
|
|
byte_after_greet = tvb_get_guint8(tvb, (gint) i); /* OK */
|
|
|
|
or
|
|
|
|
gint i;
|
|
char greeting[] = "hello, sailor";
|
|
guint byte_after_greet;
|
|
|
|
i = (gint) strlen(greeting);
|
|
byte_after_greet = tvb_get_guint8(tvb, i); /* OK */
|
|
|
|
See http://www.unix.org/version2/whatsnew/lp64_wp.html for more
|
|
information on the sizes of common types in different data models.
|
|
|
|
When printing or displaying the values of 64-bit integral data types,
|
|
don't use "%lld", "%llu", "%llx", or "%llo" - not all platforms
|
|
support "%ll" for printing 64-bit integral data types. Instead, for
|
|
GLib routines, and routines that use them, such as all the routines in
|
|
Wireshark that take format arguments, use G_GINT64_MODIFIER, for example:
|
|
|
|
proto_tree_add_uint64_format_value(tree, hf_uint64, tvb, offset, len,
|
|
val, "%" G_GINT64_MODIFIER "u", val);
|
|
|
|
When specifying an integral constant that doesn't fit in 32 bits, don't
|
|
use "LL" at the end of the constant - not all compilers use "LL" for
|
|
that. Instead, put the constant in a call to the "G_GINT64_CONSTANT()"
|
|
macro, e.g.
|
|
|
|
G_GINT64_CONSTANT(-11644473600), G_GUINT64_CONSTANT(11644473600)
|
|
|
|
rather than
|
|
|
|
-11644473600LL, 11644473600ULL
|
|
|
|
Don't assume that you can scan through a va_list initialized by va_start
|
|
more than once without closing it with va_end and re-initializing it with
|
|
va_start. This applies even if you're not scanning through it yourself,
|
|
but are calling a routine that scans through it, such as vfprintf() or
|
|
one of the routines in Wireshark that takes a format and a va_list as an
|
|
argument. You must do
|
|
|
|
va_start(ap, format);
|
|
call_routine1(xxx, format, ap);
|
|
va_end(ap);
|
|
va_start(ap, format);
|
|
call_routine2(xxx, format, ap);
|
|
va_end(ap);
|
|
|
|
rather
|
|
va_start(ap, format);
|
|
call_routine1(xxx, format, ap);
|
|
call_routine2(xxx, format, ap);
|
|
va_end(ap);
|
|
|
|
Don't use a label without a statement following it. For example,
|
|
something such as
|
|
|
|
if (...) {
|
|
|
|
...
|
|
|
|
done:
|
|
}
|
|
|
|
will not work with all compilers - you have to do
|
|
|
|
if (...) {
|
|
|
|
...
|
|
|
|
done:
|
|
;
|
|
}
|
|
|
|
with some statement, even if it's a null statement, after the label.
|
|
|
|
Don't use "bzero()", "bcopy()", or "bcmp()"; instead, use the ANSI C
|
|
routines
|
|
|
|
"memset()" (with zero as the second argument, so that it sets
|
|
all the bytes to zero);
|
|
|
|
"memcpy()" or "memmove()" (note that the first and second
|
|
arguments to "memcpy()" are in the reverse order to the
|
|
arguments to "bcopy()"; note also that "bcopy()" is typically
|
|
guaranteed to work on overlapping memory regions, while
|
|
"memcpy()" isn't, so if you may be copying from one region to a
|
|
region that overlaps it, use "memmove()", not "memcpy()" - but
|
|
"memcpy()" might be faster as a result of not guaranteeing
|
|
correct operation on overlapping memory regions);
|
|
|
|
and "memcmp()" (note that "memcmp()" returns 0, 1, or -1, doing
|
|
an ordered comparison, rather than just returning 0 for "equal"
|
|
and 1 for "not equal", as "bcmp()" does).
|
|
|
|
Not all platforms necessarily have "bzero()"/"bcopy()"/"bcmp()", and
|
|
those that do might not declare them in the header file on which they're
|
|
declared on your platform.
|
|
|
|
Don't use "index()" or "rindex()"; instead, use the ANSI C equivalents,
|
|
"strchr()" and "strrchr()". Not all platforms necessarily have
|
|
"index()" or "rindex()", and those that do might not declare them in the
|
|
header file on which they're declared on your platform.
|
|
|
|
Don't use "tvb_get_ptr(). If you must use it, keep in mind that the pointer
|
|
returned by a call to "tvb_get_ptr()" is not guaranteed to be aligned on any
|
|
particular byte boundary; this means that you cannot safely cast it to any
|
|
data type other than a pointer to "char", unsigned char", "guint8", or other
|
|
one-byte data types. Casting a pointer returned by tvb_get_ptr() into any
|
|
multi-byte data type or structure may cause crashes on some platforms (even
|
|
if it does not crash on x86-based PCs). Even if such mis-aligned accesses
|
|
don't crash on your platform they will be slower than properly aligned
|
|
accesses would be. Furthermore, the data in a packet is not necessarily in
|
|
the byte order of the machine on which Wireshark is running. Use the tvbuff
|
|
routines to extract individual items from the packet, or, better yet, use
|
|
"proto_tree_add_item()" and let it extract the items for you.
|
|
|
|
Don't use structures that overlay packet data, or into which you copy
|
|
packet data; the C programming language does not guarantee any
|
|
particular alignment of fields within a structure, and even the
|
|
extensions that try to guarantee that are compiler-specific and not
|
|
necessarily supported by all compilers used to build Wireshark. Using
|
|
bitfields in those structures is even worse; the order of bitfields
|
|
is not guaranteed.
|
|
|
|
Don't use "ntohs()", "ntohl()", "htons()", or "htonl()"; the header
|
|
files required to define or declare them differ between platforms, and
|
|
you might be able to get away with not including the appropriate header
|
|
file on your platform but that might not work on other platforms.
|
|
Instead, use "g_ntohs()", "g_ntohl()", "g_htons()", and "g_htonl()";
|
|
those are declared by <glib.h>, and you'll need to include that anyway,
|
|
as Wireshark header files that all dissectors must include use stuff from
|
|
<glib.h>.
|
|
|
|
Don't fetch a little-endian value using "tvb_get_ntohs() or
|
|
"tvb_get_ntohl()" and then using "g_ntohs()", "g_htons()", "g_ntohl()",
|
|
or "g_htonl()" on the resulting value - the g_ routines in question
|
|
convert between network byte order (big-endian) and *host* byte order,
|
|
not *little-endian* byte order; not all machines on which Wireshark runs
|
|
are little-endian, even though PCs are. Fetch those values using
|
|
"tvb_get_letohs()" and "tvb_get_letohl()".
|
|
|
|
Do not use "open()", "rename()", "mkdir()", "stat()", "unlink()", "remove()",
|
|
"fopen()", "freopen()" directly. Instead use "ws_open()", "ws_rename()",
|
|
"ws_mkdir()", "ws_stat()", "ws_unlink()", "ws_remove()", "ws_fopen()",
|
|
"ws_freopen()": these wrapper functions change the path and file name from
|
|
UTF8 to UTF16 on Windows allowing the functions to work correctly when the
|
|
path or file name contain non-ASCII characters.
|
|
|
|
Also, use ws_read(), ws_write(), ws_lseek(), ws_dup(), ws_fstat(), and
|
|
ws_fdopen(), rather than read(), write(), lseek(), dup(), fstat(), and
|
|
fdopen() on descriptors returned by ws_open().
|
|
|
|
Those functions are declared in <wsutil/file_util.h>; include that
|
|
header in any code that uses any of those routines.
|
|
|
|
When opening a file with "ws_fopen()", "ws_freopen()", or "ws_fdopen()", if
|
|
the file contains ASCII text, use "r", "w", "a", and so on as the open mode
|
|
- but if it contains binary data, use "rb", "wb", and so on. On
|
|
Windows, if a file is opened in a text mode, writing a byte with the
|
|
value of octal 12 (newline) to the file causes two bytes, one with the
|
|
value octal 15 (carriage return) and one with the value octal 12, to be
|
|
written to the file, and causes bytes with the value octal 15 to be
|
|
discarded when reading the file (to translate between C's UNIX-style
|
|
lines that end with newline and Windows' DEC-style lines that end with
|
|
carriage return/line feed).
|
|
|
|
In addition, that also means that when opening or creating a binary
|
|
file, you must use "ws_open()" (with O_CREAT and possibly O_TRUNC if the
|
|
file is to be created if it doesn't exist), and OR in the O_BINARY flag,
|
|
even on UN*X - O_BINARY is defined by <wsutil/file_util.h> as 0 on UN*X.
|
|
|
|
Do not include <unistd.h>, <fcntl.h>, or <io.h> to declare any of the
|
|
routines listed as replaced by routines in <wsutil/file_util.h>;
|
|
instead, just include <wsutil/file_util.h>.
|
|
|
|
If you need the declarations of other functions defined by <unistd.h>,
|
|
don't include it without protecting it with
|
|
|
|
#ifdef HAVE_UNISTD_H
|
|
|
|
...
|
|
|
|
#endif
|
|
|
|
Don't use forward declarations of static arrays without a specified size
|
|
in a fashion such as this:
|
|
|
|
static const value_string foo_vals[];
|
|
|
|
...
|
|
|
|
static const value_string foo_vals[] = {
|
|
{ 0, "Red" },
|
|
{ 1, "Green" },
|
|
{ 2, "Blue" },
|
|
{ 0, NULL }
|
|
};
|
|
|
|
as some compilers will reject the first of those statements. Instead,
|
|
initialize the array at the point at which it's first declared, so that
|
|
the size is known.
|
|
|
|
For #define names and enum member names, prefix the names with a tag so
|
|
as to avoid collisions with other names - this might be more of an issue
|
|
on Windows, as it appears to #define names such as DELETE and
|
|
OPTIONAL.
|
|
|
|
Don't use the "numbered argument" feature that many UNIX printf's
|
|
implement, e.g.:
|
|
|
|
g_snprintf(add_string, 30, " - (%1$d) (0x%1$04x)", value);
|
|
|
|
as not all UNIX printf's implement it, and Windows printf doesn't appear
|
|
to implement it. Use something like
|
|
|
|
g_snprintf(add_string, 30, " - (%d) (0x%04x)", value, value);
|
|
|
|
instead.
|
|
|
|
Don't use
|
|
|
|
case N ... M:
|
|
|
|
as that's not supported by all compilers.
|
|
|
|
snprintf() -> g_snprintf()
|
|
snprintf() is not available on all platforms, so it's a good idea to use the
|
|
g_snprintf() function declared by <glib.h> instead.
|
|
|
|
tmpnam() -> mkstemp()
|
|
tmpnam is insecure and should not be used any more. Wireshark brings its
|
|
own mkstemp implementation for use on platforms that lack mkstemp.
|
|
Note: mkstemp does not accept NULL as a parameter.
|
|
|
|
Wireshark supports platforms with GLib 2.22[.x]/GTK+ 2.12[.x]/Qt 4.8[.x] or
|
|
newer. If a Glib/GTK+/Qt mechanism is available only in newer Glib/GTK+/Qt
|
|
versions then use "#if GLIB_CHECK_VERSION(...)", "#if GTK_CHECK_VERSION(...)" or
|
|
"#if QT_VERSION_CHECK(...)" to conditionally compile code using that mechanism.
|
|
|
|
When different code must be used on UN*X and Win32, use a #if or #ifdef
|
|
that tests _WIN32, not WIN32. Try to write code portably whenever
|
|
possible, however; note that there are some routines in Wireshark with
|
|
platform-dependent implementations and platform-independent APIs, such
|
|
as the routines in epan/filesystem.c, allowing the code that calls it to
|
|
be written portably without #ifdefs.
|
|
|
|
Wireshark uses Libgcrypt as general-purpose crypto library. To use it from
|
|
your dissector, do not include gcrypt.h directly, but use the wrapper file
|
|
wsutil/wsgcrypt.h instead.
|
|
|
|
2. String handling
|
|
|
|
Do not use functions such as strcat() or strcpy().
|
|
A lot of work has been done to remove the existing calls to these functions and
|
|
we do not want any new callers of these functions.
|
|
|
|
Instead use g_snprintf() since that function will if used correctly prevent
|
|
buffer overflows for large strings.
|
|
|
|
Be sure that all pointers passed to %s specifiers in format strings are non-
|
|
NULL. Some implementations will automatically replace NULL pointers with the
|
|
string "(NULL)", but most will not.
|
|
|
|
When using a buffer to create a string, do not use a buffer stored on the stack.
|
|
I.e. do not use a buffer declared as
|
|
|
|
char buffer[1024];
|
|
|
|
instead allocate a buffer dynamically using the string-specific or plain wmem
|
|
routines (see README.wmem) such as
|
|
|
|
wmem_strbuf_t *strbuf;
|
|
strbuf = wmem_strbuf_new(wmem_packet_scope(), "");
|
|
wmem_strbuf_append_printf(strbuf, ...
|
|
|
|
or
|
|
|
|
char *buffer=NULL;
|
|
...
|
|
#define MAX_BUFFER 1024
|
|
buffer=wmem_alloc(wmem_packet_scope(), MAX_BUFFER);
|
|
buffer[0]='\0';
|
|
...
|
|
g_snprintf(buffer, MAX_BUFFER, ...
|
|
|
|
This avoids the stack from being corrupted in case there is a bug in your code
|
|
that accidentally writes beyond the end of the buffer.
|
|
|
|
|
|
If you write a routine that will create and return a pointer to a filled in
|
|
string and if that buffer will not be further processed or appended to after
|
|
the routine returns (except being added to the proto tree),
|
|
do not preallocate the buffer to fill in and pass as a parameter instead
|
|
pass a pointer to a pointer to the function and return a pointer to a
|
|
wmem-allocated buffer that will be automatically freed. (see README.wmem)
|
|
|
|
I.e. do not write code such as
|
|
static void
|
|
foo_to_str(char *string, ... ){
|
|
<fill in string>
|
|
}
|
|
...
|
|
char buffer[1024];
|
|
...
|
|
foo_to_str(buffer, ...
|
|
proto_tree_add_string(... buffer ...
|
|
|
|
instead write the code as
|
|
static void
|
|
foo_to_str(char **buffer, ...
|
|
#define MAX_BUFFER x
|
|
*buffer=wmem_alloc(wmem_packet_scope(), MAX_BUFFER);
|
|
<fill in *buffer>
|
|
}
|
|
...
|
|
char *buffer;
|
|
...
|
|
foo_to_str(&buffer, ...
|
|
proto_tree_add_string(... *buffer ...
|
|
|
|
Use wmem_ allocated buffers. They are very fast and nice. These buffers are all
|
|
automatically free()d when the dissection of the current packet ends so you
|
|
don't have to worry about free()ing them explicitly in order to not leak memory.
|
|
Please read README.wmem.
|
|
|
|
Don't use non-ASCII characters in source files; not all compiler
|
|
environments will be using the same encoding for non-ASCII characters,
|
|
and at least one compiler (Microsoft's Visual C) will, in environments
|
|
with double-byte character encodings, such as many Asian environments,
|
|
fail if it sees a byte sequence in a source file that doesn't correspond
|
|
to a valid character. This causes source files using either an ISO
|
|
8859/n single-byte character encoding or UTF-8 to fail to compile. Even
|
|
if the compiler doesn't fail, there is no guarantee that the compiler,
|
|
or a developer's text editor, will interpret the characters the way you
|
|
intend them to be interpreted.
|
|
|
|
3. Robustness.
|
|
|
|
Wireshark is not guaranteed to read only network traces that contain correctly-
|
|
formed packets. Wireshark is commonly used to track down networking
|
|
problems, and the problems might be due to a buggy protocol implementation
|
|
sending out bad packets.
|
|
|
|
Therefore, code does not only have to be able to handle
|
|
correctly-formed packets without, for example, crashing or looping
|
|
infinitely, they also have to be able to handle *incorrectly*-formed
|
|
packets without crashing or looping infinitely.
|
|
|
|
Here are some suggestions for making code more robust in the face
|
|
of incorrectly-formed packets:
|
|
|
|
Do *NOT* use "g_assert()" or "g_assert_not_reached()" in dissectors.
|
|
*NO* value in a packet's data should be considered "wrong" in the sense
|
|
that it's a problem with the dissector if found; if it cannot do
|
|
anything else with a particular value from a packet's data, the
|
|
dissector should put into the protocol tree an indication that the
|
|
value is invalid, and should return. The "expert" mechanism should be
|
|
used for that purpose.
|
|
|
|
If there is a case where you are checking not for an invalid data item
|
|
in the packet, but for a bug in the dissector (for example, an
|
|
assumption being made at a particular point in the code about the
|
|
internal state of the dissector), use the DISSECTOR_ASSERT macro for
|
|
that purpose; this will put into the protocol tree an indication that
|
|
the dissector has a bug in it, and will not crash the application.
|
|
|
|
If you are allocating a chunk of memory to contain data from a packet,
|
|
or to contain information derived from data in a packet, and the size of
|
|
the chunk of memory is derived from a size field in the packet, make
|
|
sure all the data is present in the packet before allocating the buffer.
|
|
Doing so means that:
|
|
|
|
1) Wireshark won't leak that chunk of memory if an attempt to
|
|
fetch data not present in the packet throws an exception.
|
|
|
|
and
|
|
|
|
2) it won't crash trying to allocate an absurdly-large chunk of
|
|
memory if the size field has a bogus large value.
|
|
|
|
If you're fetching into such a chunk of memory a string from the buffer,
|
|
and the string has a specified size, you can use "tvb_get_*_string()",
|
|
which will check whether the entire string is present before allocating
|
|
a buffer for the string, and will also put a trailing '\0' at the end of
|
|
the buffer.
|
|
|
|
If you're fetching into such a chunk of memory a 2-byte Unicode string
|
|
from the buffer, and the string has a specified size, you can use
|
|
"tvb_get_faked_unicode()", which will check whether the entire string
|
|
is present before allocating a buffer for the string, and will also
|
|
put a trailing '\0' at the end of the buffer. The resulting string will be
|
|
a sequence of single-byte characters; the only Unicode characters that
|
|
will be handled correctly are those in the ASCII range. (Wireshark's
|
|
ability to handle non-ASCII strings is limited; it needs to be
|
|
improved.)
|
|
|
|
If you're fetching into such a chunk of memory a sequence of bytes from
|
|
the buffer, and the sequence has a specified size, you can use
|
|
"tvb_memdup()", which will check whether the entire sequence is present
|
|
before allocating a buffer for it.
|
|
|
|
Otherwise, you can check whether the data is present by using
|
|
"tvb_ensure_bytes_exist()" although this frequently is not needed: the
|
|
TVB-accessor routines can handle requests to read data beyond the end of
|
|
the TVB (by throwing an exception which will either mark the frame as
|
|
truncated--not all the data was captured--or as malformed).
|
|
|
|
Note also that you should only fetch string data into a fixed-length
|
|
buffer if the code ensures that no more bytes than will fit into the
|
|
buffer are fetched ("the protocol ensures" isn't good enough, as
|
|
protocol specifications can't ensure only packets that conform to the
|
|
specification will be transmitted or that only packets for the protocol
|
|
in question will be interpreted as packets for that protocol by
|
|
Wireshark). If there's no maximum length of string data to be fetched,
|
|
routines such as "tvb_get_*_string()" are safer, as they allocate a buffer
|
|
large enough to hold the string. (Note that some variants of this call
|
|
require you to free the string once you're finished with it.)
|
|
|
|
If you have gotten a pointer using "tvb_get_ptr()" (which you should not
|
|
have: you should seriously consider a better alternative to this function),
|
|
you must make sure that you do not refer to any data past the length passed
|
|
as the last argument to "tvb_get_ptr()"; while the various "tvb_get"
|
|
routines perform bounds checking and throw an exception if you refer to data
|
|
not available in the tvbuff, direct references through a pointer gotten from
|
|
"tvb_get_ptr()" do not do any bounds checking.
|
|
|
|
If you have a loop that dissects a sequence of items, each of which has
|
|
a length field, with the offset in the tvbuff advanced by the length of
|
|
the item, then, if the length field is the total length of the item, and
|
|
thus can be zero, you *MUST* check for a zero-length item and abort the
|
|
loop if you see one. Otherwise, a zero-length item could cause the
|
|
dissector to loop infinitely. You should also check that the offset,
|
|
after having the length added to it, is greater than the offset before
|
|
the length was added to it, if the length field is greater than 24 bits
|
|
long, so that, if the length value is *very* large and adding it to the
|
|
offset causes an overflow, that overflow is detected.
|
|
|
|
If you have a
|
|
|
|
for (i = {start}; i < {end}; i++)
|
|
|
|
loop, make sure that the type of the loop index variable is large enough
|
|
to hold the maximum {end} value plus 1; otherwise, the loop index
|
|
variable can overflow before it ever reaches its maximum value. In
|
|
particular, be very careful when using gint8, guint8, gint16, or guint16
|
|
variables as loop indices; you almost always want to use an "int"/"gint"
|
|
or "unsigned int"/"guint" as the loop index rather than a shorter type.
|
|
|
|
If you are fetching a length field from the buffer, corresponding to the
|
|
length of a portion of the packet, and subtracting from that length a
|
|
value corresponding to the length of, for example, a header in the
|
|
packet portion in question, *ALWAYS* check that the value of the length
|
|
field is greater than or equal to the length you're subtracting from it,
|
|
and report an error in the packet and stop dissecting the packet if it's
|
|
less than the length you're subtracting from it. Otherwise, the
|
|
resulting length value will be negative, which will either cause errors
|
|
in the dissector or routines called by the dissector, or, if the value
|
|
is interpreted as an unsigned integer, will cause the value to be
|
|
interpreted as a very large positive value.
|
|
|
|
Any tvbuff offset that is added to as processing is done on a packet
|
|
should be stored in a 32-bit variable, such as an "int"; if you store it
|
|
in an 8-bit or 16-bit variable, you run the risk of the variable
|
|
overflowing.
|
|
|
|
sprintf() -> g_snprintf()
|
|
Prevent yourself from using the sprintf() function, as it does not test the
|
|
length of the given output buffer and might be writing into unintended memory
|
|
areas. This function is one of the main causes of security problems like buffer
|
|
exploits and many other bugs that are very hard to find. It's much better to
|
|
use the g_snprintf() function declared by <glib.h> instead.
|
|
|
|
You should test your dissector against incorrectly-formed packets. This
|
|
can be done using the randpkt and editcap utilities that come with the
|
|
Wireshark distribution. Testing using randpkt can be done by generating
|
|
output at the same layer as your protocol, and forcing Wireshark/TShark
|
|
to decode it as your protocol, e.g. if your protocol sits on top of UDP:
|
|
|
|
randpkt -c 50000 -t dns randpkt.pcap
|
|
tshark -nVr randpkt.pcap -d udp.port==53,<myproto>
|
|
|
|
Testing using editcap can be done using preexisting capture files and the
|
|
"-E" flag, which introduces errors in a capture file. E.g.:
|
|
|
|
editcap -E 0.03 infile.pcap outfile.pcap
|
|
tshark -nVr outfile.pcap
|
|
|
|
The script fuzz-test.sh is available to help automate these tests.
|
|
|
|
4. Name convention.
|
|
|
|
Wireshark uses the underscore_convention rather than the InterCapConvention for
|
|
function names, so new code should probably use underscores rather than
|
|
intercaps for functions and variable names. This is especially important if you
|
|
are writing code that will be called from outside your code. We are just
|
|
trying to keep things consistent for other developers.
|
|
|
|
C symbols exported from libraries shipped with Wireshark should start with a
|
|
prefix that helps avoiding name collision with public symbols from other shared
|
|
libraries. The current suggested prefixes for newly added symbols are
|
|
ws_, wslua_, wmem_ and wtap_.
|
|
|
|
5. White space convention.
|
|
|
|
Please avoid using tab expansions different from 8 column widths, as not all
|
|
text editors in use by the developers support this. For a detailed
|
|
discussion of tabs, spaces, and indentation, see
|
|
|
|
http://www.jwz.org/doc/tabs-vs-spaces.html
|
|
|
|
Most of the files in Wireshark tend to use 2-space or 4-space indentation. When
|
|
creating a new file you are free to choose an indentation logic but you are
|
|
encouraged to use 4-space indentation for C/C++ source to keep inconsistency
|
|
between files to a minimum.
|
|
|
|
Each file should have a short comment (modelines) on the indentation logic at
|
|
the end of the file. See
|
|
|
|
https://www.wireshark.org/tools/modelines.html
|
|
|
|
for guidance.
|
|
|
|
A complementary and arguably better alternative to modelines is
|
|
EditorConfig (http://editorconfig.org). The default EditorConfig indentation
|
|
style is defined in the .editorconfig file placed on the root of the project
|
|
source code tree (4-space indentation for C). One advantage of following the
|
|
default style is that no additional EditorConfig settings are required for new
|
|
files. If your file does not follow this style please add a file-specific
|
|
setting overriding the default style to a separate .editorconfig configuration
|
|
file placed in the source file directory.
|
|
|
|
Some IDEs and text editors have built-in EditorConfig support and many more
|
|
have plugins available (links at http://editorconfig.org).
|
|
|
|
Please do not leave trailing whitespace (spaces/tabs) on lines.
|
|
|
|
When editing an existing file, try following the existing indentation
|
|
logic and even if it very tempting, never ever use a restyler/reindenter
|
|
utility on an existing file. If you run across wildly varying
|
|
indentation styles within the same file, it might be helpful to send a
|
|
note to wireshark-dev for guidance.
|
|
|
|
6. Compiler warnings
|
|
|
|
You should write code that is free of compiler warnings. Such warnings will
|
|
often indicate questionable code and sometimes even real bugs, so it's best
|
|
to avoid warnings at all.
|
|
|
|
The compiler flags in the Makefiles are set to "treat warnings as errors",
|
|
so your code won't even compile when warnings occur.
|
|
|
|
7. Miscellaneous notes
|
|
|
|
Each commit in your branch corresponds to a different VCSVERSION string
|
|
automatically defined in the header 'version.h' during the build. If you happen
|
|
to find it convenient to disable this feature it can be done using:
|
|
|
|
touch .git/wireshark-disable-versioning
|
|
|
|
i.e., the file 'wireshark-disable-versioning' must exist in the git repo dir.
|
|
|
|
/*
|
|
* Editor modelines - https://www.wireshark.org/tools/modelines.html
|
|
*
|
|
* Local variables:
|
|
* c-basic-offset: 4
|
|
* tab-width: 8
|
|
* indent-tabs-mode: nil
|
|
* End:
|
|
*
|
|
* vi: set shiftwidth=4 tabstop=8 expandtab:
|
|
* :indentSize=4:tabSize=8:noTabs=true:
|
|
*/
|