Parfait analysis of GNU sed 4.2.2

[Rich Burridge]

Hi,

I'm in the process of updating the version of GNU sed that's in
the Oracle Solaris O/S to 4.2.2. Part of this update requires
a security audit, including running the Parfait static code analysis
tool on the GNU sed code.

For more information on parfait, see the paper:

http://labs.oracle.com/projects/downunder/publications/pepm09.pdf

Anyhoo, parfait found the following error:

Error: Buffer overrun
Buffer overflow (CWE 120): In array dereference of
((char*)(word_limit + -1))[12] with index '-1+12'
Array size is 32000 bytes, -1+12 is -20
at line 257 of components/sed/sed-4.2.2/sed/fmt.c in function
'get_paragraph'.
Read outside array bounds (CWE 125): In array dereference of
((char*)(word_limit + -1))[12] with index '-1+12'
Array size is 32000 bytes, -1+12 is -20
at line 257 of components/sed/sed-4.2.2/sed/fmt.c in function
'get_paragraph'.

You might wish to review this and see if it is a real problem that
need addressing (as opposed to a false positive).

Thanks.

[Paolo Bonzini]

It's a false positive.

Function get_line always advances word_limit by one or more, and there
is always at least one invocation of get_line after the assignment

word_limit = words;

Paolo

[Jose E. Marchesi]

I'm in the process of updating the version of GNU sed that's in
the Oracle Solaris O/S to 4.2.2. Part of this update requires
a security audit, including running the Parfait static code analysis
tool on the GNU sed code.

Hi. Thanks for reporting the static code analysis results. I will take
a look to see if they are reporting real problems.

--
Jose E. Marchesi http://www.jemarch.net
GNU Project http://www.gnu.org

[Paul Eggert]

On 02/14/13 10:10, Rich Burridge wrote:
> (word_limit + -1))[12] with index '-1+12'
> Array size is 32000 bytes, -1+12 is -20
> at line 257 of components/sed/sed-4.2.2/sed/fmt.c in function 'get_paragraph'.
>
> You might wish to review this and see if it is a real problem that
> need addressing (as opposed to a false positive).

Thanks, but these both look like false positives to me.
Among other things, -1+12 does not equal -20. I suspect it's
a bug in Parfait. Perhaps it's just a problem in how its information
is displayed, in which case it could be identifying a bug in sed,
but offhand I don't see what that bug would be.

[Jose E. Marchesi]

> You might wish to review this and see if it is a real problem that
> need addressing (as opposed to a false positive).

It's a false positive.

Function get_line always advances word_limit by one or more, and there
is always at least one invocation of get_line after the assignment

word_limit = words;

Heh, we crossed emails :) No need to investigate then.