forked from osmocom/wireshark
f9df9ffb3f
Change-Id: I15bfda76256e5d1084ad7c64006f93afaf071a9e Reviewed-on: https://code.wireshark.org/review/13010 Reviewed-by: Guy Harris <guy@alum.mit.edu>
706 lines
29 KiB
Text
706 lines
29 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.
|
|
|
|
Don't use C++-style comments (comments beginning with "//" and running
|
|
to the end of the line) in C code. Wireshark's dissectors are written in
|
|
C, and thus run through C rather than C++ compilers, and not all C
|
|
compilers support C++-style comments (GCC does, but IBM's C compiler for
|
|
AIX, for example, doesn't do so by default). C++-style comments can be
|
|
used in C++ code, of course.
|
|
|
|
In general, don't use C99 features 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:
|
|
|
|
variadic macros
|
|
|
|
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; not all compilers support them. If an
|
|
array would have no members, just leave it out.
|
|
|
|
Don't declare variables in the middle of executable code; not all C
|
|
compilers support that. Variables should be declared outside a
|
|
function, or at the beginning of a function or compound statement.
|
|
|
|
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()".
|
|
|
|
Don't put a comma after the last element of an enum - some compilers may
|
|
either warn about it (producing extra noise) or refuse to accept it.
|
|
|
|
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.
|
|
|
|
Don't put a comma after the last tuple of an initializer of an array.
|
|
|
|
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.14[.x]/GTK+ 2.12[.x] or newer.
|
|
If a Glib/GTK+ mechanism is available only in Glib/GTK+ versions newer
|
|
than 2.14/2.12 then use "#if GLIB_CHECK_VERSION(...)" or "#if
|
|
GTK_CHECK_VERSION(...)" 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, protect libgcrypt calls with #ifdef HAVE_LIBGCRYPT. Don't
|
|
include gcrypt.h directly, include 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.
|
|
|
|
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
|
|
|
|
When creating a new file, you are free to choose an indentation logic.
|
|
Most of the files in Wireshark tend to use 2-space or 4-space
|
|
indentation. You are encouraged to write a short comment on the
|
|
indentation logic at the beginning of this new files. The
|
|
tabs-vs-spaces document above provides examples of Emacs and vi
|
|
modelines for this purpose.
|
|
|
|
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.
|
|
|
|
/*
|
|
* 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:
|
|
*/
|