Re: [PATCH v3] staging: android: Replace strcpy with strlcpy
From: Greg KH
Date: Sun Mar 12 2017 - 11:04:52 EST
On Sun, Mar 12, 2017 at 08:25:28PM +0530, SIMRAN SINGHAL wrote:
> On Sun, Mar 12, 2017 at 7:04 PM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > On Sun, Mar 12, 2017 at 03:32:44AM +0530, simran singhal wrote:
> >> Replace strcpy with strlcpy as strcpy does not check for buffer
> >> overflow.
> >
> > Can there be a buffer overflow here? If not, then strcpy is just fine
> > to use. Do you see a potential code path here that actually is a
> > problem using this?
> >
> >> This is found using Flawfinder.
> >
> > You mean 'grep'? :)
> >
> > If not, what exactly does "Flawfinder" point out is wrong with the code
> > here? At first glance, I can't find it, but perhaps the tool, and your
> > audit, provided more information?
> >
> > thanks,
> >
>
> Flawfinder reports possible security weaknesses (âflawsâ) sorted by risk level.
> The risk level is shown inside square brackets and varies from 0, very
> little risk,
> to 5, great risk.
>
> So, here in this case I was getting risk of [4].
> This is what I got:
> drivers/staging/android/ashmem.c:551: [4] (buffer) strcpy:
> Does not check for buffer overflows when copying to destination (CWE-120).
> Consider using strcpy_s, strncpy, or strlcpy (warning, strncpy is easily
> misused).
Consider looking at the code to see if it actually is incorrect before
blindly accepting random comments by a random tool :)
Again, if you can see how this is incorrect, great, let's fix it,
otherwise please leave it as-is because so far your fixes are actually
breaking things :(
thanks,
greg k-h