Wireshark mailing list archives

Re: Considering ignoring Coverity 'tainted' checks


From: Dario Lombardo <dario.lombardo.ml () gmail com>
Date: Mon, 11 Jul 2016 15:20:39 +0200

I can't answer for all the "tainted" checks on coverity, but the ones I'm
patching seem reasonable.

Quick example

num = tvb_get_ntohl(...)
for (i = 0; i < num; i++) { }

Nothing is going to crash in wireshark, but if a tainted packet contains
0xFFFFFFFF, why should we loop with this value, until we overcome the
boudaries? The value is clearly wrong. My idea of patch is to check this
value against a max, and stop the current call if the check fails.

Currently there are 160 tainted checks open in coverity. How can we be sure
that all of them are unuseful, hence we disable the check? What about look
at them, and maybe mark some them as false positive?

On Mon, Jul 11, 2016 at 12:46 PM, Jaap Keuter <jaap.keuter () xs4all nl> wrote:

Hi List,

Since (not so) recently the Coverity code analysis has added a checker for
so called tainted data. This data is considered coming from an external
source (eg. the network) hence suspicious until validated. Using these
tainted values is considered a risk. In general this is true, Wireshark on
the other hand is intended and designed to handle suspicious / (very)
possibly wrong network data (that’s what we’re using it for, amongst other
things). So even though data is tainted, many cases the use of the TVB,
etc. protects us from the problems envisioned by the checker writers.

So what to so with these Coverity issues. Before we start to implement all
kinds of arbitrary checks (duplicating effort already handled by the tvb
code) and limits (mostly arbitrary) we should consider is this checker is
really valuable in this context.

Kind regards,
Jaap

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org
?subject=unsubscribe
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

Current thread: