1
0
mirror of https://github.com/postgres/postgres.git synced 2025-08-15 14:02:29 +03:00
Commit Graph

4007 Commits

Author SHA1 Message Date
Tom Lane
a411361705 Stamp 10.13. 2020-05-11 17:12:38 -04:00
Michael Paquier
1bb0293d84 Fix memory leak in libpq when using sslmode=verify-full
Checking if Subject Alternative Names (SANs) from a certificate match
with the hostname connected to leaked memory after each lookup done.

This is broken since acd08d7 that added support for SANs in SSL
certificates, so backpatch down to 9.5.

Author: Roman Peshkurov
Reviewed-by: Hamid Akhtar, Michael Paquier, David Steele
Discussion: https://postgr.es/m/CALLDf-pZ-E3mjxd5=bnHsDu9zHEOnpgPgdnO84E2RuwMCjjyPw@mail.gmail.com
Backpatch-through: 9.5
2020-04-22 07:27:53 +09:00
Tom Lane
95d2576d02 Stamp 10.12. 2020-02-10 17:19:45 -05:00
Peter Eisentraut
7f97b5e891 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: fc4d6b13b6a369c443a8f573f3836c5759d1a919
2020-02-10 13:00:18 +01:00
Tom Lane
d09cfa3e28 libpq should expose GSS-related parameters even when not implemented.
We realized years ago that it's better for libpq to accept all
connection parameters syntactically, even if some are ignored or
restricted due to lack of the feature in a particular build.
However, that lesson from the SSL support was for some reason never
applied to the GSSAPI support.  This is causing various buildfarm
members to have problems with a test case added by commit 6136e94dc,
and it's just a bad idea from a user-experience standpoint anyway,
so fix it.

While at it, fix some places where parameter-related infrastructure
was added with the aid of a dartboard, or perhaps with the aid of
the anti-pattern "add new stuff at the end".  It should be safe
to rearrange the contents of struct pg_conn even in released
branches, since that's private to libpq (and we'd have to move
some fields in some builds to fix this, anyway).

Back-patch to all supported branches.

Discussion: https://postgr.es/m/11297.1576868677@sss.pgh.pa.us
2019-12-20 15:34:08 -05:00
Tomas Vondra
f71b22f537 Fix off-by-one error in PGTYPEStimestamp_fmt_asc
When using %b or %B patterns to format a date, the code was simply using
tm_mon as an index into array of month names. But that is wrong, because
tm_mon is 1-based, while array indexes are 0-based. The result is we
either use name of the next month, or a segfault (for December).

Fix by subtracting 1 from tm_mon for both patterns, and add a regression
test triggering the issue. Backpatch to all supported versions (the bug
is there far longer, since at least 2003).

Reported-by: Paul Spencer
Backpatch-through: 9.4
Discussion: https://postgr.es/m/16143-0d861eb8688d3fef%40postgresql.org
2019-11-30 15:07:47 +01:00
Tom Lane
004ae0a976 Stamp 10.11. 2019-11-11 17:07:14 -05:00
Peter Eisentraut
96bcc335a3 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: f052c053b7992cde6b854c5ab1b51055f4506133
2019-11-11 10:33:28 +01:00
Tom Lane
831ca9513c Move declaration of ecpg_gettext() to a saner place.
Declaring this in the client-visible header ecpglib.h was a pretty
poor decision.  It's not meant to be application-callable (and if
it was, putting it outside the extern "C" { ... } wrapper means
that C++ clients would fail to call it).  And the declaration would
not even compile for a client, anyway, since it would not have the
macro pg_attribute_format_arg().  Fortunately, it seems that no
clients have tried to include this header with ENABLE_NLS defined,
or we'd have gotten complaints about that.  But we have no business
putting such a restriction on client code.

Move the declaration to ecpglib_extern.h, since in fact nothing
outside src/interfaces/ecpg/ecpglib/ needs to call it.

The practical effect of this is just that clients can now safely
#include ecpglib.h while having ENABLE_NLS defined, but that seems
like enough of a reason to back-patch it.

Discussion: https://postgr.es/m/20590.1573069709@sss.pgh.pa.us
2019-11-07 14:21:52 -05:00
Tom Lane
2fbecb66c4 Stamp 10.10. 2019-08-05 17:16:54 -04:00
Peter Eisentraut
5e32ae9d05 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 361643d9eb7a2585dfcfc73838fee5a9940db6eb
2019-08-05 15:44:33 +02:00
Tom Lane
cb65b454f8 Fix failures to ignore \r when reading Windows-style newlines.
libpq failed to ignore Windows-style newlines in connection service files.
This normally wasn't a problem on Windows itself, because fgets() would
convert \r\n to just \n.  But if libpq were running inside a program that
changes the default fopen mode to binary, it would see the \r's and think
they were data.  In any case, it's project policy to ignore \r in text
files unconditionally, because people sometimes try to use files with
DOS-style newlines on Unix machines, where the C library won't hide that
from us.

Hence, adjust parseServiceFile() to ignore \r as well as \n at the end of
the line.  In HEAD, go a little further and make it ignore all trailing
whitespace, to match what it's always done with leading whitespace.

In HEAD, also run around and fix up everyplace where we have
newline-chomping code to make all those places look consistent and
uniformly drop \r.  It is not clear whether any of those changes are
fixing live bugs.  Most of the non-cosmetic changes are in places that
are reading popen output, and the jury is still out as to whether popen
on Windows can return \r\n.  (The Windows-specific code in pipe_read_line
seems to think so, but our lack of support for this elsewhere suggests
maybe it's not a problem in practice.)  Hence, I desisted from applying
those changes to back branches, except in run_ssl_passphrase_command()
which is new enough and little-tested enough that we'd probably not have
heard about any problems there.

Tom Lane and Michael Paquier, per bug #15827 from Jorge Gustavo Rocha.
Back-patch the parseServiceFile() change to all supported branches,
and the run_ssl_passphrase_command() change to v11 where that was added.

Discussion: https://postgr.es/m/15827-e6ba53a3a7ed543c@postgresql.org
2019-07-25 12:11:21 -04:00
Tom Lane
cb4cb7401e Stamp 10.9. 2019-06-17 17:18:49 -04:00
Michael Paquier
d72a7e4da1 Fix buffer overflow when processing SCRAM final message in libpq
When a client connects to a rogue server sending specifically-crafted
messages, this can suffice to execute arbitrary code as the operating
system account used by the client.

While on it, fix one error handling when decoding an incorrect salt
included in the first message received from server.

Author: Michael Paquier
Reviewed-by: Jonathan Katz, Heikki Linnakangas
Security: CVE-2019-10164
Backpatch-through: 10
2019-06-17 22:14:09 +09:00
Tom Lane
8de574aa8b Attempt to identify system timezone by reading /etc/localtime symlink.
On many modern platforms, /etc/localtime is a symlink to a file within the
IANA database.  Reading the symlink lets us find out the name of the system
timezone directly, without going through the brute-force search embodied in
scan_available_timezones().  This shortens the runtime of initdb by some
tens of ms, which is helpful for the buildfarm, and it also allows us to
reliably select the same zone name the system was actually configured for,
rather than possibly choosing one of IANA's many zone aliases.  (For
example, in a system configured for "Asia/Tokyo", the brute-force search
would not choose that name but its alias "Japan", on the grounds of the
latter string being shorter.  More surprisingly, "Navajo" is preferred
to either "America/Denver" or "US/Mountain", as seen in an old complaint
from Josh Berkus.)

If /etc/localtime doesn't exist, or isn't a symlink, or we can't make
sense of its contents, or the contents match a zone we know but that
zone doesn't match the observed behavior of localtime(), fall back to
the brute-force search.

Also, tweak initdb so that it prints the zone name it selected.

In passing, replace the last few references to the "Olson" database in
code comments with "IANA", as that's been our preferred term since
commit b2cbced9e.

Back-patch of commit 23bd3cec6.  The original intention was to not
back-patch, since this can result in cosmetic behavioral changes ---
for example, on my own workstation initdb now chooses "America/New_York",
where it used to prefer "US/Eastern" which is equivalent and shorter.
However, our hand has been more or less forced by tzdb update 2019a,
which made the "UCT" zone fully equivalent to "UTC".  Our old code
now prefers "UCT" on the grounds of it being alphabetically first,
and that's making nobody happy.  Choosing the alias indicated by
/etc/localtime is a more defensible behavior.  (Users who don't like
the results can always force the decision by setting the TZ environment
variable before running initdb.)

Patch by me, per a suggestion from Robert Haas; review by Michael Paquier

Discussion: https://postgr.es/m/7408.1525812528@sss.pgh.pa.us
Discussion: https://postgr.es/m/20190604085735.GD24018@msg.df7cb.de
2019-06-14 11:25:13 -04:00
Tom Lane
e3bf3c0f8c Fix misuse of an integer as a bool.
pgtls_read_pending is declared to return bool, but what the underlying
SSL_pending function returns is a count of available bytes.

This is actually somewhat harmless if we're using C99 bools, but in
the back branches it's a live bug: if the available-bytes count happened
to be a multiple of 256, it would get converted to a zero char value.
On machines where char is signed, counts of 128 and up could misbehave
as well.  The net effect is that when using SSL, libpq might block
waiting for data even though some has already been received.

Broken by careless refactoring in commit 4e86f1b16, so back-patch
to 9.5 where that came in.

Per bug #15802 from David Binderman.

Discussion: https://postgr.es/m/15802-f0911a97f0346526@postgresql.org
2019-05-13 10:53:19 -04:00
Tom Lane
4411902898 Stamp 10.8. 2019-05-06 16:48:45 -04:00
Peter Eisentraut
f4540a9c9c Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: f80b00b1a9a01653acd9f814badda4b2ed0e48f1
2019-05-06 14:43:35 +02:00
Michael Meskes
e7e71b9e1e Fix off-by-one check that can lead to a memory overflow in ecpg.
Patch by Liu Huailing <liuhuailing@cn.fujitsu.com>
2019-04-11 21:05:39 +02:00
Michael Meskes
5469a1e78c Fix potential memory access violation in ecpg if filename of include file is
shorter than 2 characters.

Patch by: "Wu, Fei" <wufei.fnst@cn.fujitsu.com>
2019-03-11 16:15:09 +01:00
Tom Lane
8ec638e69c Fix ecpg bugs caused by missing semicolons in the backend grammar.
The Bison documentation clearly states that a semicolon is required
after every grammar rule, and our scripts that generate ecpg's
grammar from the backend's implicitly assumed this is true.  But it
turns out that only ancient versions of Bison actually enforce that.
There have been a couple of rules without trailing semicolons in
gram.y for some time, and as a consequence, ecpg's grammar was faulty
and produced wrong output for the affected statements.

To fix, add the missing semis, and add some cross-checks to ecpg's
scripts so that they'll bleat if we mess this up again.

The cases that were broken were:
* "SET variable = DEFAULT" (but not "SET variable TO DEFAULT"),
  as well as allied syntaxes such as ALTER SYSTEM SET ... DEFAULT.
  These produced syntactically invalid output that the server
  would reject.
* Multiple type names in DROP TYPE/DOMAIN commands.  Only the
  first type name would be listed in the emitted command.

Per report from Daisuke Higuchi.  Back-patch to all supported versions.

Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905DB51CE@g01jpexmbkw24
2019-02-24 12:51:50 -05:00
Tom Lane
3b940cd9dc Fix omissions in ecpg/test/sql/.gitignore.
Oversights in commits 050710b36 and e81f0e311.
2019-02-18 21:24:38 -05:00
Michael Meskes
f647a5fb0d Sync ECPG's CREATE TABLE AS statement with backend's.
Author: Higuchi-san ("Higuchi, Daisuke" <higuchi.daisuke@jp.fujitsu.com>)
2019-02-18 12:05:14 +01:00
Tom Lane
a093e14449 Stamp 10.7. 2019-02-11 16:19:36 -05:00
Peter Eisentraut
ff3ac5903d Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: da5fe0060d012477d807c1b60f1ff2188947a72f
2019-02-11 14:25:01 +01:00
Tom Lane
5b5cb3b0eb Remove infinite-loop hazards in ecpg test suite.
A report from Andrew Dunstan showed that an ecpglib breakage that
causes repeated query failures could lead to infinite loops in some
ecpg test scripts, because they contain "while(1)" loops with no
exit condition other than successful test completion.  That might
be all right for manual testing, but it seems entirely unacceptable
for automated test environments such as our buildfarm.  We don't
want buildfarm owners to have to intervene manually when a test
goes wrong.

To fix, just change all those while(1) loops to exit after at most
100 iterations (which is more than any of them expect to iterate).
This seems sufficient since we'd see discrepancies in the test output
if any loop executed the wrong number of times.

I tested this by dint of intentionally breaking ecpg_do_prologue
to always fail, and verifying that the tests still got to completion.

Back-patch to all supported branches, since the whole point of this
exercise is to protect the buildfarm against future mistakes.

Discussion: https://postgr.es/m/18693.1548302004@sss.pgh.pa.us
2019-01-24 16:46:55 -05:00
Tom Lane
dd815a94cc Blind attempt to fix _configthreadlocale() failures on MinGW.
Apparently, some builds of MinGW contain a version of
_configthreadlocale() that always returns -1, indicating failure.
Rather than treating that as a curl-up-and-die condition, soldier on
as though the function didn't exist.  This leaves us without thread
safety on such MinGW versions, but we didn't have it anyway.

Discussion: https://postgr.es/m/d06a16bc-52d6-9f0d-2379-21242d7dbe81@2ndQuadrant.com
2019-01-23 22:47:03 -05:00
Tom Lane
6106f9dd71 Avoid thread-safety problem in ecpglib.
ecpglib attempts to force the LC_NUMERIC locale to "C" while reading
server output, to avoid problems with strtod() and related functions.
Historically it's just issued setlocale() calls to do that, but that
has major problems if we're in a threaded application.  setlocale()
itself is not required by POSIX to be thread-safe (and indeed is not,
on recent OpenBSD).  Moreover, its effects are process-wide, so that
we could cause unexpected results in other threads, or another thread
could change our setting.

On platforms having uselocale(), which is required by POSIX:2008,
we can avoid these problems by using uselocale() instead.  Windows
goes its own way as usual, but we can make it safe by using
_configthreadlocale().  Platforms having neither continue to use the
old code, but that should be pretty much nobody among current systems.

(Subsequent buildfarm results show that recent NetBSD versions still
lack uselocale(), but it's not a big problem because they also do not
support non-"C" settings for LC_NUMERIC.)

Back-patch of commits 8eb4a9312 and ee27584c4.

Michael Meskes and Tom Lane; thanks also to Takayuki Tsunakawa.

Discussion: https://postgr.es/m/31420.1547783697@sss.pgh.pa.us
2019-01-21 23:18:58 -05:00
Tom Lane
2e8ed46599 Second try at fixing numeric data passed through an ECPG SQLDA.
In commit ecfd55795, I removed sqlda.c's checks for ndigits != 0 on the
grounds that we should duplicate the state of the numeric value's digit
buffer even when all the digits are zeroes.  However, that still isn't
quite right, because another possible state of the digit buffer is
buf == digits == NULL (this occurs for a NaN).  As the code now stands,
it'll invoke memcpy with a NULL source address and zero bytecount,
which we know a few platforms crash on.  Hence, reinstate the no-copy
short-circuit, but make it test specifically for buf != NULL rather than
some other condition.  In hindsight, the ndigits test (added by commit
f2ae9f9c3) was almost certainly meant to fix the NaN case not the
all-zeroes case as the associated thread alleged.

As before, back-patch to all supported versions.

Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905C71161@g01jpexmbkw24
2018-11-14 11:27:30 -05:00
Tom Lane
32060f6780 Fix incorrect results for numeric data passed through an ECPG SQLDA.
Numeric values with leading zeroes were incorrectly copied into a
SQLDA (SQL Descriptor Area), leading to wrong results in ECPG programs.

Report and patch by Daisuke Higuchi.  Back-patch to all supported
versions.

Discussion: https://postgr.es/m/1803D792815FC24D871C00D17AE95905C71161@g01jpexmbkw24
2018-11-13 15:46:08 -05:00
Tom Lane
c63d9ebb59 Stamp 10.6. 2018-11-05 16:45:50 -05:00
Peter Eisentraut
5d846a2dd7 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 4aac9391521d21fdecc378db4750a59795350b33
2018-11-05 14:46:40 +01:00
Tom Lane
ecc59e31a8 Client-side fixes for delayed NOTIFY receipt.
PQnotifies() is defined to just process already-read data, not try to read
any more from the socket.  (This is a debatable decision, perhaps, but I'm
hesitant to change longstanding library behavior.)  The documentation has
long recommended calling PQconsumeInput() before PQnotifies() to ensure
that any already-arrived message would get absorbed and processed.
However, psql did not get that memo, which explains why it's not very
reliable about reporting notifications promptly.

Also, most (not quite all) callers called PQconsumeInput() just once before
a PQnotifies() loop.  Taking this recommendation seriously implies that we
should do PQconsumeInput() before each call.  This is more important now
that we have "payload" strings in notification messages than it was before;
that increases the probability of having more than one packet's worth
of notify messages.  Hence, adjust code as well as documentation examples
to do it like that.

Back-patch to 9.5 to match related server fixes.  In principle we could
probably go back further with these changes, but given lack of field
complaints I doubt it's worthwhile.

Discussion: https://postgr.es/m/CAOYf6ec-TmRYjKBXLLaGaB-jrd=mjG1Hzn1a1wufUAR39PQYhw@mail.gmail.com
2018-10-19 22:22:57 -04:00
Peter Eisentraut
6592d89068 ecpg: Change --version output to common style
When we removed the ecpg-specific versions, we also removed the
"(PostgreSQL)" from the --version output, which we show in other
programs.

Reported-by: Ioseph Kim <pgsql-kr@postgresql.kr>
2018-09-12 14:34:53 +02:00
Tom Lane
930b785d40 Minor cleanup/future-proofing for pg_saslprep().
Ensure that pg_saslprep() initializes its output argument to NULL in
all failure paths, and then remove the redundant initialization that
some (not all) of its callers did.  This does not fix any live bug,
but it reduces the odds of future bugs of omission.

Also add a comment about why the existing failure-path coding is
adequate.

Back-patch so as to keep the function's API consistent across branches,
again to forestall future bug introduction.

Patch by me, reviewed by Michael Paquier

Discussion: https://postgr.es/m/16558.1536407783@sss.pgh.pa.us
2018-09-08 18:20:36 -04:00
Andrew Gierth
d64fad6669 Fix lexing of standard multi-character operators in edge cases.
Commits c6b3c939b (which fixed the precedence of >=, <=, <> operators)
and 865f14a2d (which added support for the standard => notation for
named arguments) created a class of lexer tokens which look like
multi-character operators but which have their own token IDs distinct
from Op. However, longest-match rules meant that following any of
these tokens with another operator character, as in (1<>-1), would
cause them to be incorrectly returned as Op.

The error here isn't immediately obvious, because the parser would
usually still find the correct operator via the Op token, but there
were more subtle problems:

1. If immediately followed by a comment or +-, >= <= <> would be given
   the old precedence of Op rather than the correct new precedence;

2. If followed by a comment, != would be returned as Op rather than as
   NOT_EQUAL, causing it not to be found at all;

3. If followed by a comment or +-, the => token for named arguments
   would be lexed as Op, causing the argument to be mis-parsed as a
   simple expression, usually causing an error.

Fix by explicitly checking for the operators in the {operator} code
block in addition to all the existing special cases there.

Backpatch to 9.5 where the problem was introduced.

Analysis and patch by me; review by Tom Lane.
Discussion: https://postgr.es/m/87va851ppl.fsf@news-spur.riddles.org.uk
2018-08-23 21:43:55 +01:00
Andrew Gierth
2dbfbd630b Reduce an unnecessary O(N^3) loop in lexer.
The lexer's handling of operators contained an O(N^3) hazard when
dealing with long strings of + or - characters; it seems hard to
prevent this case from being O(N^2), but the additional N multiplier
was not needed.

Backpatch all the way since this has been there since 7.x, and it
presents at least a mild hazard in that trying to do Bind, PREPARE or
EXPLAIN on a hostile query could take excessive time (without
honouring cancels or timeouts) even if the query was never executed.
2018-08-23 21:43:55 +01:00
Tom Lane
6953daf08e In libpq, don't look up all the hostnames at once.
Historically, we looked up the target hostname in connectDBStart, so that
PQconnectPoll did not need to do DNS name resolution.  The patches that
added multiple-target-host support to libpq preserved this division of
labor; but it's really nonsensical now, because it means that if any one
of the target hosts fails to resolve in DNS, the connection fails.  That
negates the no-single-point-of-failure goal of the feature.  Additionally,
DNS lookups aren't exactly cheap, but the code did them all even if the
first connection attempt succeeds.

Hence, rearrange so that PQconnectPoll does the lookups, and only looks
up a hostname when it's time to try that host.  This does mean that
PQconnectPoll could block on a DNS lookup --- but if you wanted to avoid
that, you should be using hostaddr, as the documentation has always
specified.  It seems fairly unlikely that any applications would really
care whether the lookup occurs inside PQconnectStart or PQconnectPoll.

In addition to calling out that fact explicitly, do some other minor
wordsmithing in the docs around the multiple-target-host feature.

Since this seems like a bug in the multiple-target-host feature,
backpatch to v10 where that was introduced.  In the back branches,
avoid moving any existing fields of struct pg_conn, just in case
any third-party code is looking into that struct.

Tom Lane, reviewed by Fabien Coelho

Discussion: https://postgr.es/m/4913.1533827102@sss.pgh.pa.us
2018-08-23 16:39:20 -04:00
Tom Lane
6101bc2f45 Clean up assorted misuses of snprintf()'s result value.
Fix a small number of places that were testing the result of snprintf()
but doing so incorrectly.  The right test for buffer overrun, per C99,
is "result >= bufsize" not "result > bufsize".  Some places were also
checking for failure with "result == -1", but the standard only says
that a negative value is delivered on failure.

(Note that this only makes these places correct if snprintf() delivers
C99-compliant results.  But at least now these places are consistent
with all the other places where we assume that.)

Also, make psql_start_test() and isolation_start_test() check for
buffer overrun while constructing their shell commands.  There seems
like a higher risk of overrun, with more severe consequences, here
than there is for the individual file paths that are made elsewhere
in the same functions, so this seemed like a worthwhile change.

Also fix guc.c's do_serialize() to initialize errno = 0 before
calling vsnprintf.  In principle, this should be unnecessary because
vsnprintf should have set errno if it returns a failure indication ...
but the other two places this coding pattern is cribbed from don't
assume that, so let's be consistent.

These errors are all very old, so back-patch as appropriate.  I think
that only the shell command overrun cases are even theoretically
reachable in practice, but there's not much point in erroneous error
checks.

Discussion: https://postgr.es/m/17245.1534289329@sss.pgh.pa.us
2018-08-15 16:29:32 -04:00
Tom Lane
e0db288abf Fix libpq's implementation of per-host connection timeouts.
Commit 5f374fe7a attempted to turn the connect_timeout from an overall
maximum time limit into a per-host limit, but it didn't do a great job of
that.  The timer would only get restarted if we actually detected timeout
within connectDBComplete(), not if we changed our attention to a new host
for some other reason.  In that case the old timeout continued to run,
possibly causing a premature timeout failure for the new host.

Fix that, and also tweak the logic so that if we do get a timeout,
we advance to the next available IP address, not to the next host name.
There doesn't seem to be a good reason to assume that all the IP
addresses supplied for a given host name will necessarily fail the
same way as the current one.  Moreover, this conforms better to the
admittedly-vague documentation statement that the timeout is "per
connection attempt".  I changed that to "per host name or IP address"
to be clearer.  (Note that reconnections to the same server, such as for
switching protocol version or SSL status, don't get their own separate
timeout; that was true before and remains so.)

Also clarify documentation about the interpretation of connect_timeout
values less than 2.

This seems like a bug, so back-patch to v10 where this logic came in.

Tom Lane, reviewed by Fabien Coelho

Discussion: https://postgr.es/m/5735.1533828184@sss.pgh.pa.us
2018-08-13 13:07:53 -04:00
Tom Lane
4191e37a9a Stamp 10.5. 2018-08-06 16:05:31 -04:00
Peter Eisentraut
4ae1121423 Translation updates
Source-Git-URL: https://git.postgresql.org/git/pgtranslation/messages.git
Source-Git-Hash: 3463878fc340cb1436153b18a300f8cfdcb12adb
2018-08-06 20:03:55 +02:00
Tom Lane
ab5400469b Fix failure to reset libpq's state fully between connection attempts.
The logic in PQconnectPoll() did not take care to ensure that all of
a PGconn's internal state variables were reset before trying a new
connection attempt.  If we got far enough in the connection sequence
to have changed any of these variables, and then decided to try a new
server address or server name, the new connection might be completed
with some state that really only applied to the failed connection.

While this has assorted bad consequences, the only one that is clearly
a security issue is that password_needed didn't get reset, so that
if the first server asked for a password and the second didn't,
PQconnectionUsedPassword() would return an incorrect result.  This
could be leveraged by unprivileged users of dblink or postgres_fdw
to allow them to use server-side login credentials that they should
not be able to use.

Other notable problems include the possibility of forcing a v2-protocol
connection to a server capable of supporting v3, or overriding
"sslmode=prefer" to cause a non-encrypted connection to a server that
would have accepted an encrypted one.  Those are certainly bugs but
it's harder to paint them as security problems in themselves.  However,
forcing a v2-protocol connection could result in libpq having a wrong
idea of the server's standard_conforming_strings setting, which opens
the door to SQL-injection attacks.  The extent to which that's actually
a problem, given the prerequisite that the attacker needs control of
the client's connection parameters, is unclear.

These problems have existed for a long time, but became more easily
exploitable in v10, both because it introduced easy ways to force libpq
to abandon a connection attempt at a late stage and then try another one
(rather than just giving up), and because it provided an easy way to
specify multiple target hosts.

Fix by rearranging PQconnectPoll's state machine to provide centralized
places to reset state properly when moving to a new target host or when
dropping and retrying a connection to the same host.

Tom Lane, reviewed by Noah Misch.  Our thanks to Andrew Krasichkov
for finding and reporting the problem.

Security: CVE-2018-10915
2018-08-06 10:53:35 -04:00
Tom Lane
8d00858baf Change libpq's internal uses of PQhost() to inspect host field directly.
Commit 1944cdc98 changed PQhost() to return the hostaddr value when that
is specified and host isn't.  This is a good idea in general, but
fe-auth.c and related files contain PQhost() calls for which it isn't.
Specifically, when we compare SSL certificates or other server identity
information to the host field, we do not want to use hostaddr instead;
that's not what's documented, that's not what happened pre-v10, and
it doesn't seem like a good idea.

Instead, we can just look at connhost[].host directly.  This does what
we want in v10 and up; in particular, if neither host nor hostaddr
were given, the host field will be replaced with the default host name.
That seems useful, and it's likely the reason that these places were
coded to call PQhost() originally (since pre-v10, the stored field was
not replaced with the default).

Back-patch to v10, as 1944cdc98 (just) was.

Discussion: https://postgr.es/m/23287.1533227021@sss.pgh.pa.us
2018-08-03 12:12:10 -04:00
Tom Lane
62038810b7 libpq: PQhost to return active connected host or hostaddr
Previously, PQhost didn't return the connected host details when the
connection type was CHT_HOST_ADDRESS (i.e., via hostaddr).  Instead, it
returned the complete host connection parameter (which could contain
multiple hosts) or the default host details, which was confusing and
arguably incorrect.

Change this to return the actually connected host or hostaddr
irrespective of the connection type.  When hostaddr but no host was
specified, hostaddr is now returned.  Never return the original host
connection parameter, and document that PQhost cannot be relied on
before the connection is established.

PQport is similarly changed to always return the active connection port
and never the original connection parameter.

Back-patch of commit 1944cdc982
into the v10 branch.

Author: Hari Babu <kommi.haribabu@gmail.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Reviewed-by: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
2018-08-03 11:30:34 -04:00
Tom Lane
71e3b28901 Fix libpq's code for searching .pgpass; rationalize empty-list-item cases.
Before v10, we always searched ~/.pgpass using the host parameter,
and nothing else, to match to the "hostname" field of ~/.pgpass.
(However, null host or host matching DEFAULT_PGSOCKET_DIR was replaced by
"localhost".)  In v10, this got broken by commit 274bb2b38, repaired by
commit bdac9836d, and broken again by commit 7b02ba62e; in the code
actually shipped, we'd search with hostaddr if both that and host were
specified --- though oddly, *not* if only hostaddr were specified.
Since this is directly contrary to the documentation, and not
backwards-compatible, it's clearly a bug.

However, the change wasn't totally without justification, even though it
wasn't done quite right, because the pre-v10 behavior has arguably been
buggy since we added hostaddr.  If hostaddr is specified and host isn't,
the pre-v10 code will search ~/.pgpass for "localhost", and ship that
password off to a server that most likely isn't local at all.  That's
unhelpful at best, and could be a security breach at worst.

Therefore, rather than just revert to that old behavior, let's define
the behavior as "search with host if provided, else with hostaddr if
provided, else search for localhost".  (As before, a host name matching
DEFAULT_PGSOCKET_DIR is replaced by localhost.)  This matches the
behavior of the actual connection code, so that we don't pick up an
inappropriate password; and it allows useful searches to happen when
only hostaddr is given.

While we're messing around here, ensure that empty elements within a
host or hostaddr list select the same behavior as a totally-empty
field would; for instance "host=a,,b" is equivalent to "host=a,/tmp,b"
if DEFAULT_PGSOCKET_DIR is /tmp.  Things worked that way in some cases
already, but not consistently so, which contributed to the confusion
about what key ~/.pgpass would get searched with.

Update documentation accordingly, and also clarify some nearby text.

Back-patch to v10 where the host/hostaddr list functionality was
introduced.

Discussion: https://postgr.es/m/30805.1532749137@sss.pgh.pa.us
2018-08-01 12:30:36 -04:00
Heikki Linnakangas
ff4fb4cc1b Fix error message when a hostaddr cannot be parsed.
We were incorrectly passing hostname, not hostaddr, in the error message,
and because of that, you got:

$ psql 'hostaddr=foo'
psql: could not parse network address "(null)": Name or service not known

Backpatch to v10, where this was broken (by commit 7b02ba62e9).

Report and fix by Robert Haas.

Discussion: https://www.postgresql.org/message-id/CA+TgmoapFQA30NomGKEaZCu3iN7mF7fux8fbbk9SouVOT2JP7w@mail.gmail.com
2018-07-19 20:25:05 +03:00
Heikki Linnakangas
ed529faf7e Fix misc typos, mostly in comments.
A collection of typos I happened to spot while reading code, as well as
grepping for common mistakes.

Backpatch to all supported versions, as applicable, to avoid conflicts
when backporting other commits in the future.
2018-07-18 16:18:27 +03:00
Tom Lane
c74f48a4ec Prevent accidental linking of system-supplied copies of libpq.so etc.
Back-patch commit dddfc4cb2, which broke LDFLAGS and related Makefile
variables into two parts, one for within-build-tree library references and
one for external libraries, to ensure that the order of -L flags has all
of the former before all of the latter.  This turns out to fix a problem
recently noted on buildfarm member peripatus, that we attempted to
incorporate code from libpgport.a into a shared library.  That will fail on
platforms that are sticky about putting non-PIC code into shared libraries.
(It's quite surprising we hadn't seen such failures before, since the code
in question has been like that for a long time.)

I think that peripatus' problem could have been fixed with just a subset
of this patch; but since the previous issue of accidentally linking to the
wrong copy of a Postgres shlib seems likely to bite people in the field,
let's just back-patch the whole change.  Now that commit dddfc4cb2 has
survived some beta testing, I'm less afraid to back-patch it than I was
at the time.

This also fixes undesired inclusion of "-DFRONTEND" in pg_config's CPPFLAGS
output (in 9.6 and up) and undesired inclusion of "-L../../src/common" in
its LDFLAGS output (in all supported branches).

Back-patch to v10 and older branches; this is already in v11.

Discussion: https://postgr.es/m/20180704234304.bq2dxispefl65odz@ler-imac.local
2018-07-09 17:23:31 -04:00
Thomas Munro
3566873f21 Add PGTYPESchar_free() to avoid cross-module problems on Windows.
On Windows, it is sometimes important for corresponding malloc() and
free() calls to be made from the same DLL, since some build options can
result in multiple allocators being active at the same time.  For that
reason we already provided PQfreemem().  This commit adds a similar
function for freeing string results allocated by the pgtypes library.

Author: Takayuki Tsunakawa
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/0A3221C70F24FB45833433255569204D1F8AD5D6%40G01JPEXMBYT05
2018-06-26 19:49:52 +12:00