Re: Static Code Analysis Exploits.

Patrick Curran <pcurran@contentanalyst.com> writes:
> We use Postgres in our product and we have a client that requires a 
> static code analysis scan to detect vulnerabilities. They are concerned 
> because the tool (Veracode) found several flaws in Postgres and they 
> believe there might be a security risk. I'm sure there are lots of 
> companies that use Postgres that have security policies like theirs in 
> place, so I'm hoping someone has the experience to know that these are 
> false positives or that they are not a security risk for some reason. 
> Below is a description of the vulnerability and the location in the 
> source code. Version 9.3.2.1 was scanned. Please let me know if there is 
> a better place to ask this kind of question.

TBH, I don't think anyone's going to bother with going through this list
in this form.  Line numbers in something that's not an official community
release are not very helpful, and the descriptions are far too vague for
someone looking at the list to be sure exactly what their tool is on
about.  I took one example at random:

> Stack-based Buffer Overflow (CWE ID 121)(13 flaws):
> There is a potential buffer overflow with these functions. If an 
> attacker can control the data written into the buffer, the overflow may 
> result in execution of arbitrary code.

> libpq.dll .../interfaces/libpq/pqexpbuffer.c 369

This seems to be complaining about the memcpy in appendBinaryPQExpBuffer.
Well, I don't see anything unsafe about it: the preceding call to
enlargePQExpBuffer should have made sure that the target buffer is big
enough.  And the reference to stack-based buffer overflow is completely
nonsensical, because no PQExpBuffer keeps its buffer on the stack.  It's
conceivable that their tool has spotted some unsafe pattern in some call
site, but this report is unhelpful about identifying what that would be.

I did look at a few of the other items, and none of the ones I looked at
were any more clear.

FWIW, we do have access to Coverity scans of the Postgres sources, and
we do make efforts to fix things Coverity complains about.  But we're
unlikely to take reports like this one seriously: there's simply not
enough information to know what the tool is unhappy about, nor any
clear reason to believe that it's spotted something that both human
readers and Coverity have missed.

Sorry if that's not the answer you wanted; but a more positive response
is going to require a substantially greater amount of work on your end.
In particular, given the very substantial amounts of work that have
already gone into hardening the Postgres code, I think the burden of
proof is on you or your client to show that these are issues, not on
us to disprove claims that are too vague to be disproven.
        regards, tom lane