- Feb 21, 2015
-
-
Tom Lane authored
After finding an "=" character, the pointer was advanced twice when it should only advance once. This is harmless as long as the value after "=" has at least one character; but if it doesn't, we'd miss the terminator character and include too much in the value. In principle this could lead to reading off the end of memory. It does not seem worth treating as a security issue though, because it would happen on client side, and besides client logic that's taking conninfo strings from untrusted sources has much worse security problems than this. Report and patch received off-list from Thomas Fanghaenel. Back-patch to 9.2 where the faulty code was introduced.
-
- Feb 18, 2015
-
-
Tom Lane authored
cfopen() and cfopen_write() failed to pass the compression level through to zlib, so that you always got the default compression level if you got any at all. In passing, also fix these and related functions so that the correct errno is reliably returned on failure; the original coding supposes that free() cannot change errno, which is untrue on at least some platforms. Per bug #12779 from Christoph Berg. Back-patch to 9.1 where the faulty code was introduced. Michael Paquier
-
- Feb 17, 2015
-
-
Tom Lane authored
In investigating yesterday's crash report from Hugo Osvaldo Barrera, I only looked back as far as commit f3aec2c7 where the breakage occurred (which is why I thought the IPv4-in-IPv6 business was undocumented). But actually the logic dates back to commit 3c9bb888 and was simply broken by erroneous refactoring in the later commit. A bit of archives excavation shows that we added the whole business in response to a report that some 2003-era Linux kernels would report IPv4 connections as having IPv4-in-IPv6 addresses. The fact that we've had no complaints since 9.0 seems to be sufficient confirmation that no modern kernels do that, so let's just rip it all out rather than trying to fix it. Do this in the back branches too, thus essentially deciding that our effective behavior since 9.0 is correct. If there are any platforms on which the kernel reports IPv4-in-IPv6 addresses as such, yesterday's fix would have made for a subtle and potentially security-sensitive change in the effective meaning of IPv4 pg_hba.conf entries, which does not seem like a good thing to do in minor releases. So let's let the post-9.0 behavior stand, and change the documentation to match it. In passing, I failed to resist the temptation to wordsmith the description of pg_hba.conf IPv4 and IPv6 address entries a bit. A lot of this text hasn't been touched since we were IPv4-only.
-
Robert Haas authored
Avoid losing errno if readdir() fails and closedir() works. This also avoids leaking the directory handle when readdir() fails. Commit 6f03927f introduced logic to better handle readdir() and closedir() failures, bu it missed these cases. Extracted from a larger patch by Marco Nenciarini.
-
Andres Freund authored
I bungled resolving a conflict while backpatching 2c0a4858 to 9.2, by passing mark_done = true to ReceiveXlogStream in pg_receivexlog.c (all the other branches are ok). Since pg_receivexlog doesn't use a archive directory that causes 'could not create archive status file "...": No such file or directory' errors. Until 9.2.11 is released this can be worked around by creating 'archive_directory' in pg_receivexlog's target directory. Found by Sergey Konoplev.
-
- Feb 16, 2015
-
-
Tom Lane authored
The previous coding copied garbage into a local variable, pretty much ensuring that the intended test of an IPv6 connection address against a promoted IPv4 address from pg_hba.conf would never match. The lack of field complaints likely indicates that nobody realized this was supposed to work, which is unsurprising considering that no user-facing docs suggest it should work. In principle this could have led to a SIGSEGV due to reading off the end of memory, but since the source address would have pointed to somewhere in the function's stack frame, that's quite unlikely. What led to discovery of the bug is Hugo Osvaldo Barrera's report of a crash after an OS upgrade, which is probably because he is now running a system in which memcpy raises abort() upon detecting overlapping source and destination areas. (You'd have to additionally suppose some things about the stack frame layout to arrive at this conclusion, but it seems plausible.) This has been broken since the code was added, in commit f3aec2c7, so back-patch to all supported branches.
-
Tom Lane authored
In commit bf7ca158 I introduced an assumption that an RTE referenced by a whole-row Var must have a valid eref field. This is false for RTEs constructed by DoCopy, and there are other places taking similar shortcuts. Perhaps we should make all those places go through addRangeTableEntryForRelation or its siblings instead of having ad-hoc logic, but the most reliable fix seems to be to make the new code in ExecEvalWholeRowVar cope if there's no eref. We can reasonably assume that there's no need to insert column aliases if no aliases were provided. Add a regression test case covering this, and also verifying that a sane column name is in fact available in this situation. Although the known case only crashes in 9.4 and HEAD, it seems prudent to back-patch the code change to 9.2, since all the ingredients for a similar failure exist in the variant patch applied to 9.3 and 9.2. Per report from Jean-Pierre Pelletier.
-
- Feb 15, 2015
-
-
Peter Eisentraut authored
Before, it was writing the processed files into the input directory, which is incorrect in a vpath build.
-
- Feb 13, 2015
-
-
Heikki Linnakangas authored
Rob Rowan. Backpatch to all supported versions, like the patch that added the broken #ifdef.
-
- Feb 12, 2015
-
-
Bruce Momjian authored
This allows the delete script to properly function when special characters appear in directory paths, e.g. spaces. Backpatch through 9.0
-
Bruce Momjian authored
pg_database.datfrozenxid and pg_database.datminmxid were not preserved for the 'postgres' and 'template1' databases. This could cause missing clog file errors on access to user tables and indexes after upgrades in these databases. Backpatch through 9.0
-
Tom Lane authored
We'd leak the ident_serv data structure if the second pg_getaddrinfo_all (the one for the local address) failed. This is not of great consequence because a failure return here just leads directly to backend exit(), but if this function is going to try to clean up after itself at all, it should not have such holes in the logic. Try to fix it in a future-proof way by having all the failure exits go through the same cleanup path, rather than "optimizing" some of them. Per Coverity. Back-patch to 9.2, which is as far back as this patch applies cleanly.
-
Tom Lane authored
We already had one go at this issue in commit d73b7f97, but we failed to notice that buildACLCommands also leaked several PQExpBuffers along with a simply malloc'd string. This time let's try to make the fix a bit more future-proof by eliminating the separate exit path. It's still not exactly critical because pg_dump will curl up and die on failure; but since the amount of the potential leak is now several KB, it seems worth back-patching as far as 9.2 where the previous fix landed. Per Coverity, which evidently is smarter than clang's static analyzer.
-
- Feb 11, 2015
-
-
Michael Meskes authored
When ecpg was rewritten to the new protocol version not all variable types were corrected. This patch rewrites the code for these types to fix that. It also fixes the documentation to correctly tell the status of array handling.
-
Tom Lane authored
Back in 2003 we had a discussion about how to decide which casts to dump. At the time pg_dump really only considered an object's containing schema to decide what to dump (ie, dump whatever's not in pg_catalog), and so we chose a complicated idea involving whether the underlying types were to be dumped (cf commit a6790ce8). But users are allowed to create casts between built-in types, and we failed to dump such casts. Let's get rid of that heuristic, which has accreted even more ugliness since then, in favor of just looking at the cast's OID to decide if it's a built-in cast or not. In passing, also fix some really ancient code that supposed that it had to manufacture a dependency for the cast on its cast function; that's only true when dumping from a pre-7.3 server. This just resulted in some wasted cycles and duplicate dependency-list entries with newer servers, but we might as well improve it. Per gripes from a number of people, most recently Greg Sabino Mullane. Back-patch to all supported branches.
-
Tom Lane authored
Back in commit 400e2c93 I rewrote GEQO's gimme_tree function to improve its heuristic for modifying the given tour into a legal join order. In what can only be called a fit of hubris, I supposed that this new heuristic would *always* find a legal join order, and ripped out the old logic that allowed gimme_tree to sometimes fail. The folly of this is exposed by bug #12760, in which the "greedy" clumping behavior of merge_clump() can lead it into a dead end which could only be recovered from by un-clumping. We have no code for that and wouldn't know exactly what to do with it if we did. Rather than try to improve the heuristic rules still further, let's just recognize that it *is* a heuristic and probably must always have failure cases. So, put back the code removed in the previous commit to allow for failure (but comment it a bit better this time). It's possible that this code was actually fully correct at the time and has only been broken by the introduction of LATERAL. But having seen this example I no longer have much faith in that proposition, so back-patch to all supported branches.
-
- Feb 06, 2015
-
-
Heikki Linnakangas authored
When beginning streaming replication, the client usually issues the IDENTIFY_SYSTEM command, which used to return the current WAL insert position. That's not suitable for the intended purpose of that field, however. pg_receivexlog uses it to start replication from the reported point, but if it hasn't been flushed to disk yet, it will fail. Change IDENTIFY_SYSTEM to report the flush position instead. Backpatch to 9.1 and above. 9.0 doesn't report any WAL position.
-
- Feb 04, 2015
-
-
Andres Freund authored
On windows _isnan() (which isnan() is redirected to in port/win32.h) is declared in float.h, not math.h. Per buildfarm animal currawong. Backpatch to all supported branches.
-
- Feb 02, 2015
-
-
Tom Lane authored
Add entries for security issues. Security: CVE-2015-0241 through CVE-2015-0244
-
Heikki Linnakangas authored
If any error occurred while we were in the middle of reading a protocol message from the client, we could lose sync, and incorrectly try to interpret a part of another message as a new protocol message. That will usually lead to an "invalid frontend message" error that terminates the connection. However, this is a security issue because an attacker might be able to deliberately cause an error, inject a Query message in what's supposed to be just user data, and have the server execute it. We were quite careful to not have CHECK_FOR_INTERRUPTS() calls or other operations that could ereport(ERROR) in the middle of processing a message, but a query cancel interrupt or statement timeout could nevertheless cause it to happen. Also, the V2 fastpath and COPY handling were not so careful. It's very difficult to recover in the V2 COPY protocol, so we will just terminate the connection on error. In practice, that's what happened previously anyway, as we lost protocol sync. To fix, add a new variable in pqcomm.c, PqCommReadingMsg, that is set whenever we're in the middle of reading a message. When it's set, we cannot safely ERROR out and continue running, because we might've read only part of a message. PqCommReadingMsg acts somewhat similarly to critical sections in that if an error occurs while it's set, the error handler will force the connection to be terminated, as if the error was FATAL. It's not implemented by promoting ERROR to FATAL in elog.c, like ERROR is promoted to PANIC in critical sections, because we want to be able to use PG_TRY/CATCH to recover and regain protocol sync. pq_getmessage() takes advantage of that to prevent an OOM error from terminating the connection. To prevent unnecessary connection terminations, add a holdoff mechanism similar to HOLD/RESUME_INTERRUPTS() that can be used hold off query cancel interrupts, but still allow die interrupts. The rules on which interrupts are processed when are now a bit more complicated, so refactor ProcessInterrupts() and the calls to it in signal handlers so that the signal handlers always call it if ImmediateInterruptOK is set, and ProcessInterrupts() can decide to not do anything if the other conditions are not met. Reported by Emil Lenngren. Patch reviewed by Noah Misch and Andres Freund. Backpatch to all supported versions. Security: CVE-2015-0244
-
Noah Misch authored
This covers alterations to buffer sizing and zeroing made between imath 1.3 and imath 1.20. Valgrind Memcheck identified the buffer overruns and reliance on uninitialized data; their exploit potential is unknown. Builds specifying --with-openssl are unaffected, because they use the OpenSSL BIGNUM facility instead of imath. Back-patch to 9.0 (all supported versions). Security: CVE-2015-0243
-
Noah Misch authored
Most callers pass a stack buffer. The ensuing stack smash can crash the server, and we have not ruled out the viability of attacks that lead to privilege escalation. Back-patch to 9.0 (all supported versions). Marko Tiikkaja Security: CVE-2015-0243
-
Bruce Momjian authored
Prevent port/snprintf() from overflowing its local fixed-size buffer and pad to the desired number of digits with zeros, even if the precision is beyond the ability of the native sprintf(). port/snprintf() is only used on systems that lack a native snprintf(). Reported by Bruce Momjian. Patch by Tom Lane. Backpatch to all supported versions. Security: CVE-2015-0242
-
Bruce Momjian authored
Previously very long localized month and weekday strings could overflow the allocated buffers, causing a server crash. Reported and patch reviewed by Noah Misch. Backpatch to all supported versions. Security: CVE-2015-0241
-
Bruce Momjian authored
Previously very long field masks for floats could access memory beyond the existing buffer allocated to hold the result. Reported by Andres Freund and Peter Geoghegan. Backpatch to all supported versions. Security: CVE-2015-0241
-
Tom Lane authored
The variable name isn't optional --- looks like a copy-and-paste-o from the \set command, where it is. Dilip Kumar
-
Peter Eisentraut authored
Source-Git-URL: git://git.postgresql.org/git/pgtranslation/messages.git Source-Git-Hash: 47199c5813321fac9558fe6629fcb48989a28e44
-
Peter Eisentraut authored
The previous wording claimed that the file was always in /etc, but of course this varies with the installation layout. Write instead that it can be found via `pg_config --sysconfdir`. Even though this is still somewhat incorrect because it doesn't account of moved installations, it at least conveys that the location depends on the installation.
- Feb 01, 2015
-
-
Tom Lane authored
-
Tom Lane authored
"ECHO all" is ignored for interactive input, and has been for a very long time, though possibly not for as long as the documentation has claimed the opposite. Fix that, and also note that empty lines aren't echoed, which while dubious is another longstanding behavior (it's embedded in our regression test files for one thing). Per bug #12721 from Hans Ginzel. In HEAD, also improve the code comments in this area, and suppress an unnecessary fflush(stdout) when we're not echoing. That would likely be safe to back-patch, but I'll not risk it mere hours before a release wrap.
-
- Jan 31, 2015
-
-
Tom Lane authored
DST law changes in Chile and Mexico (state of Quintana Roo). Historical changes for Iceland.
-
- Jan 30, 2015
-
-
Tom Lane authored
Coverity points out that mdc_finish returns a pointer to a local buffer (which of course is gone as soon as the function returns), leaving open a risk of misbehaviors possibly as bad as a stack overwrite. In reality, the only possible call site is in process_data_packets() which does not examine the returned pointer at all. So there's no live bug, but nonetheless the code is confusing and risky. Refactor to avoid the issue by letting process_data_packets() call mdc_finish() directly instead of going through the pullf_read() API. Although this is only cosmetic, it seems good to back-patch so that the logic in pgp-decrypt.c stays in sync across all branches. Marko Kreen
-
Stephen Frost authored
In 804b6b6d we modified BuildIndexValueDescription to pay attention to which columns are visible to the user, but unfortunatley that commit neglected to consider indexes which are built on expressions. Handle error-reporting of violations of constraint indexes based on expressions by not returning any detail when the user does not have table-level SELECT rights. Backpatch to 9.0, as the prior commit was. Pointed out by Tom.
-
Tom Lane authored
connectby() didn't adequately check that the constructed SQL query returns what it's expected to; in fact, since commit 08c33c42 it wasn't checking that at all. This could result in a null-pointer-dereference crash if the constructed query returns only one column instead of the expected two. Less excitingly, it could also result in surprising data conversion failures if the constructed query returned values that were not I/O-conversion-compatible with the types specified by the query calling connectby(). In all branches, insist that the query return at least two columns; this seems like a minimal sanity check that can't break any reasonable use-cases. In HEAD, insist that the constructed query return the types specified by the outer query, including checking for typmod incompatibility, which the code never did even before it got broken. This is to hide the fact that the implementation does a conversion to text and back; someday we might want to improve that. In back branches, leave that alone, since adding a type check in a minor release is more likely to break things than make people happy. Type inconsistencies will continue to work so long as the actual type and declared type are I/O representation compatible, and otherwise will fail the same way they used to. Also, in all branches, be on guard for NULL results from the constructed query, which formerly would cause null-pointer dereference crashes. We now print the row with the NULL but don't recurse down from it. In passing, get rid of the rather pointless idea that build_tuplestore_recursively() should return the same tuplestore that's passed to it. Michael Paquier, adjusted somewhat by me
-
- Jan 29, 2015
-
-
Andres Freund authored
GetLockConflicts() has for a long time not properly terminated the returned array. During normal processing the returned array is zero initialized which, while not pretty, is sufficient to be recognized as a invalid virtual transaction id. But the HotStandby case is more than aesthetically broken: The allocated (and reused) array is neither zeroed upon allocation, nor reinitialized, nor terminated. Not having a terminating element means that the end of the array will not be recognized and that recovery conflict handling will thus read ahead into adjacent memory. Only terminating when hitting memory content that looks like a invalid virtual transaction id. Luckily this seems so far not have caused significant problems, besides making recovery conflict more expensive. Discussion: 20150127142713.GD29457@awork2.anarazel.de Backpatch into all supported branches.
-
Heikki Linnakangas authored
When gin_fuzzy_search_limit was used, we could jump out of startScan() without calling startScanKey(). That was harmless in 9.3 and below, because startScanKey()() didn't do anything interesting, but in 9.4 it initializes information needed for skipping entries (aka GIN fast scans), and you readily get a segfault if it's not done. Nevertheless, it was clearly wrong all along, so backpatch all the way to 9.1 where the early return was introduced. (AFAICS startScanKey() did nothing useful in 9.3 and below, because the fields it initialized were already initialized in ginFillScanKey(), but I don't dare to change that in a minor release. ginFillScanKey() is always called in gingetbitmap() even though there's a check there to see if the scan keys have already been initialized, because they never are; ginrescan() free's them.) In the passing, remove unnecessary if-check from the second inner loop in startScan(). We already check in the first loop that the condition is true for all entries. Reported by Olaf Gawenda, bug #12694, Backpatch to 9.1 and above, although AFAICS it causes a live bug only in 9.4.
-
- Jan 28, 2015
-
-
Stephen Frost authored
Commit 804b6b6d added the build of a range table in copy.c to initialize the EState es_range_table since it can be needed in error paths. Unfortunately, that commit didn't appreciate that some code paths might end up not initializing the rte which is used to build the range table. Fix that and clean up a couple others things along the way- build it only once and don't explicitly set it on the !is_from path as it doesn't make any sense there (cstate is palloc0'd, so this isn't an issue from an initializing standpoint either). The prior commit went back to 9.0, but this only goes back to 9.1 as prior to that the range table build happens immediately after building the RTE and therefore doesn't suffer from this issue. Pointed out by Robert.
-
Stephen Frost authored
While building error messages to return to the user, BuildIndexValueDescription, ExecBuildSlotValueDescription and ri_ReportViolation would happily include the entire key or entire row in the result returned to the user, even if the user didn't have access to view all of the columns being included. Instead, include only those columns which the user is providing or which the user has select rights on. If the user does not have any rights to view the table or any of the columns involved then no detail is provided and a NULL value is returned from BuildIndexValueDescription and ExecBuildSlotValueDescription. Note that, for key cases, the user must have access to all of the columns for the key to be shown; a partial key will not be returned. Back-patch all the way, as column-level privileges are now in all supported versions. This has been assigned CVE-2014-8161, but since the issue and the patch have already been publicized on pgsql-hackers, there's no point in trying to hide this commit.
-
- Jan 27, 2015
-
-
Tom Lane authored
Commit 14534353 arranged to store numeric NaN values as short-header numerics, but the field access macros did not get the memo: they thought only "SHORT" numerics have short headers. Most of the time this makes no difference because we don't access the weight or dscale of a NaN; but numeric_send does that. As pointed out by Andrew Gierth, this led to fetching uninitialized bytes. AFAICS this could not have any worse consequences than that; in particular, an unaligned stored numeric would have been detoasted by PG_GETARG_NUMERIC, so that there's no risk of a fetch off the end of memory. Still, the code is wrong on its own terms, and it's not hard to foresee future changes that might expose us to real risks. So back-patch to all affected branches.
-