Coverity: Out of bounds access

[hroest]

According to coverity, there is an out of bounds access here

https://github.com/cbielow/OpenMS/blob/IDPEP%2B%2B/src/openms/source/MATH/MISC/NNLS/NNLS.cpp?pr=%2FOpenMS%2FOpenMS%2Fpull%2F726#L253

1079394 Out-of-bounds access
109. callee_ptr_arith: Passing "&dummy" to function "OpenMS::NNLS::h12_[...]" which uses it as an array. This might corrupt or misinterpret adjacent memory locations.

@cbielow can you comment on this issue?

[cbielow]

you've probably seen the doc header of this file, its automagically
generated.
So its really hard to track whats going on.
One would need to insert some debug code and run many instances of NNLS,
or sit down and think hard.

Am 3/8/2014 1:49 PM, schrieb Hannes Roest:

According to coverity, there is an out of bounds access here

https://github.com/cbielow/OpenMS/blob/IDPEP%2B%2B/src/openms/source/MATH/MISC/NNLS/NNLS.cpp?pr=%2FOpenMS%2FOpenMS%2Fpull%2F726#L253

1079394 Out-of-bounds access
109. callee_ptr_arith: Passing "&dummy" to function
"OpenMS::NNLS::h12_[...]" which uses it as an array. This might
corrupt or misinterpret adjacent memory locations.

@cbielow https://github.com/cbielow can you comment on this issue?

—
Reply to this email directly or view it on GitHub
#727.

[hroest]

yes, but to me it seems like a pretty clear case of something being wrong

static doublereal dummy, unorm, ztest;
h12_(... &dummy, ...);

int h12_( ... doublereal *c__,  ... )
{
...
--c__; // always executed
...
sm = c__[i2] * *up;
...
}

I dont see a world where this would ever work. dummy is a double that is never initialized, then a pointer to that double gets treated as a pointer to an array, it gets decremented which looks rather bad and then the array gets accessed in multiple places (even though I am not sure whether this code then gets executed or not).

[cbielow]

true, but still the question is, how does it get invoked.
the author seems to have been aware of the fact that the variable is a
"dummy", which hints at the fact that it might not execute "sm = ..."

Am 3/8/2014 5:50 PM, schrieb Hannes Roest:

yes, but to me it seems like a pretty clear case of something being wrong

|static doublereal dummy, unorm, ztest;
h12_(... &dummy, ...);

int h12_( ... doublereal *c__, ... )
{
...
--c__; // always executed
...
sm = c__[i2] * *up;
...
}
|

I dont see a world where this would ever work. dummy is a double that
is never initialized, then a pointer to that double gets treated as a
pointer to an array, it gets decremented which looks rather bad and
then the array gets accessed in multiple places (even though I am not
sure whether this code then gets executed or not).

—
Reply to this email directly or view it on GitHub
#727 (comment).

[timosachsenberg]

Is this the original code? Maybe we can get more out of it
https://trac.lcsr.jhu.edu/cisstNetlib/browser/trunk/cisstNetlib/lawson-hanson/fortran/nnls.f?rev=9
https://trac.lcsr.jhu.edu/cisstNetlib/browser/trunk/cnetlib/lawson-hanson/fortran/h12.f

[timosachsenberg]

For the dummy call:
CALL H12 (1,NPP1,NPP1+1,M,A(1,J),1,UP,DUMMY,1,1,0)
NCV is zero.

C NCV NUMBER OF VECTORS IN C() TO BE TRANSFORMED. IF NCV .LE. 0 NO OPERATIONS WILL BE DONE ON C().

this seems to be true as we have IF (NCV.LE.0) RETURN in line 67 of h12.f

[hroest]

well it seems not worth the effort to try to figure out this code. Apparently it is working at the moment for us and our tests pass, so maybe we can just leave it be? Maybe add a comment about the Coverity error at the appropriate line which explains why we did not change the code?