On Fri, Jul 13, 2018 at 2:04 PM Jeff King <[email protected]> wrote:
>
> On Fri, Jul 13, 2018 at 01:58:05PM -0700, Stefan Beller wrote:
>
> > > I'm sad that this strcpy() wasn't caught in review. IMHO we should avoid
> > > that function altogether, even when we _think_ it can't trigger an
> > > overflow. That's easier to reason about (and makes auditing easier).
> >
> > Can we somehow automatically find "bad code" either in pathces
> > or in new code (such as pu), e.g. as a coccicheck for these functions?
>
> I'd be happy to declare strcpy() totally banned (and it more or less
> is). I found this with a simple "git grep", though it seems like a
> trivial application of coccinelle to find it. The question is what to
> convert it into.
into some "meta BUG("your review process failed")"? :-)
> xsnprintf() is often a good choice, but not always
> (e.g., if the destination isn't an array, we'd have to get the size from
> somewhere else).
>
> I wouldn't be surprised if there's a way to ask coccinelle to convert
> the easy cases and barf with an error on the hard cases or something. I
> don't know the tool very well.
I was just suggesting that tool as it is run on pu by some automation,
hence it would not fall through the cracks. I mean how often to you
happen to run git grep looking for strcpy on our code base and do we
want to rely on that in the long run?
Stefan
