On Jul 11, 2016, at 3:46 AM, Jaap Keuter <[email protected]> wrote:
> 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. For what it's worth, CIDs 1363031 and 1363032, for example, have complaints about "tainted" data, and they were real bugs. The Coverity reports did *not* do a good job of reporting the *real* problems for those two CIDs, which is that the code was assuming that some item's length field had a value <= X, for some value of X, and wasn't bothering to check whether it was > X and reporting an error in that case rather than blithely treating (X - value) as if it were the length of the remaining data even if it happens to be negative. However, if the complaint is just due to Coverity not realizing that we *are* doing the necessary checks, we should find some way to tell Coverity "trust us". ___________________________________________________________________________ Sent via: Wireshark-dev mailing list <[email protected]> Archives: https://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:[email protected]?subject=unsubscribe
