On 12/17/2014 09:26 AM, Niels de Vos wrote:
On Wed, Dec 17, 2014 at 02:26:55AM -0500, Krishnan Parthasarathi wrote:
I was looking into a Coverity issue (CID 1228603) in GlusterFS.
I sent a patch[1] before I fully understood why this was an issue.
After searching around in the internet for explanations, I identified that
the core issue was that a character buffer, storing parts of a file (external I/O),
was marked tainted. This taint spread wherever the buffer was used. This seems
acceptable in the context of static analysis. How do we indicate to Coverity that
the 'taint' would cause no harm as speculated?
[1] - Coverity fix attempt: http://review.gluster.org/#/c/9286/
[2] - CID 1228603: Use of untrusted scalar value (TAINTED_SCALAR):
glusterd-utils.c: 2131 in glusterd_readin_file()
If you visit https://scan.coverity.com/projects/987 you can request an
account and make yourself owner of this CID (enter it in the upper right
corner after clicking 'view defects').
I agree that this is safe usage. Please mark this as 'intentional'.
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.
Wouldn't it be better to make GF_ASSERT() to really stop the program ?
(GF_ASSERT() should only be used for things that should never happen
under any circumstance, and happening means something very bad has
happened). Or at least use structures like this:
if (type <= x) {
array[x] = y;
} else {
GF_ASSERT(type <= x);
}
Xavi
_______________________________________________
Gluster-devel mailing list
Gluster-devel@xxxxxxxxxxx
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
