Обсуждение: Static Code Analysis Exploits.
Hi, 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. Thanks, Patrick ------------------------------------ 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. btree_gist.dll .../btree_gist/btree_utils_num.c 115 btree_gist.dll .../btree_gist/btree_utils_num.c 123 pgcrypto.dll .../contrib/pgcrypto/crypt-md5.c 103 libpq.dll .../interfaces/libpq/fe-connect.c 3185 libpq.dll .../interfaces/libpq/fe-connect.c 3220 clusterdb.exe .../interfaces/libpq/fe-connect.c 3243 libpq.dll .../libpq/fe-protocol3.c 1661 libecpg_compat.dll .../ecpg/compatlib/informix.c 978 pgcrypto.dll .../contrib/pgcrypto/mbuf.c 112 pgcrypto.dll .../contrib/pgcrypto/mbuf.c 290 pgcrypto.dll .../contrib/pgcrypto/mbuf.c 306 pgcrypto.dll .../contrib/pgcrypto/mbuf.c 330 libpq.dll .../interfaces/libpq/pqexpbuffer.c 369 Use of Inherently Dangerous Function (CWE ID 242)(1 flaw): These functions are inherently unsafe because they does not perform bounds checking on the size of their input. An attacker can send overly long input and overflow the destination buffer, potentially resulting in execution of arbitrary code. pg_isolation_regress.exe .../src/test/regress/pg_regress.c 2307 Integer Overflow or Wraparound (CWE ID 190)(1 flaw): An integer overflow condition exists when an integer that has not been properly sanity checked is used in the determination of an offset or size for memory allocation, copying, concatenation, or similarly. If the integer in question is incremented past the maximum possible value, it may wrap to become a very small, negative number, therefore providing an unintended value. This occurs most commonly in arithmetic operations or loop iterations. Integer overflows can often result in buffer overflows or data corruption, both of which may be potentially exploited to execute arbitrary code. dict_snowball.dll .../libstemmer/utilities.c 371 Process Control (CWE ID 114)(4 flaws) A function call could result in a process control attack. An argument to a process control function is either derived from an untrusted source or is hard-coded, both of which may allow an attacker to execute malicious code under certain conditions. If an attacker is allowed to specify all or part of the filename, it may be possible to load arbitrary libraries. If the location is hard-coded and an attacker is able to place a malicious copy of the library higher in the search order than the file the application intends to load, then the application will load the malicious version. psql.exe .../src/bin/psql/print.c 752 psql.exe .../src/bin/psql/print.c 791 psql.exe .../src/bin/psql/print.c 2209 psql.exe .../src/bin/psql/print.c 2500
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
On 03/07/2014 07:19 PM, Tom Lane wrote: > 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 I understand. It makes perfect sense. Thanks for your response. It's actually been quite helpful. Thanks again, Patrick