Re: Help needed with Coverity - How to remove tainted_data_argument?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/17/2014 12:46 PM, Krishnan Parthasarathi wrote:


----- Original Message -----
On 12/17/2014 11:29 AM, Niels de Vos wrote:
On Wed, Dec 17, 2014 at 10:24:46AM +0100, Xavier Hernandez wrote:
I think the root cause of this particular problem is a pattern like this:

      GF_ASSERT(type <= x);

      array[type] = y

I think there are several places where this pattern is used. GF_ASSERT()
is
there to detect bugs, however it does not stop the execution, it simply
logs
the problem. So if there's really a bug and 'type' > x, we will do an
incorrect access into the array. Additionally, if 'type' is obtained from
a
tainted pointer, coverity considers this a potential security issue.

This is also applicable to NULL pointer checks and some others.

This issue is because we are reading contents of a file into a fixed
sized buffer. Coverity considers external sources like I/O as taint by
default. My original question was if there are ways by which we can
programmatically let Coverity know that the data is _not_ tainted. This could
be a Coverity primitive, like explained here https://communities.coverity.com/thread/2303,
or refactoring our code. AFAICS, this pattern is not the same as boundary checks.
(NB. buffer in question is fixed length array and fgets (I/O) was performed
with sizeof (buffer) as the limit.)

This problem derives to a boundary check as a side effect.

fgets() is safe (at least to avoid buffer overruns), what could not be safe is the data it reads. The problem here is that gluster allocates buffers with a header for memory accounting. The sequence is this:

1. fgets() gets data into a buffer. The buffer pointer is marked as tainted.
2. A buffer is allocated to store a copy of the string, so the resulting pointer is also marked as tainted. 3. The string is assigned to an array of strings. This seems to mark the array of strings as tainted (maybe this shouldn't happen). 4. The array of strings is redimensioned. This generates an access to the memory area just before the pointer to read memory block header info. Since this access has been made using a tainted pointer, the data read is also marked as tainted. Basically this means that 'type' is considered unsafe. 5. 'type' is used to access an array. Since 'type' is considered unsafe, the access itself could be unsafe.

Anyway I agree that this isn't a bug. It seems more a limitation of the tracking of what is really tainted and what not in coverity scan.

Xavi
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://supercolony.gluster.org/mailman/listinfo/gluster-devel




[Index of Archives]     [Gluster Users]     [Ceph Users]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux