Be more careful to not lose sync in the FE/BE protocol

Enterprise / PostgreSQL - Heikki Linnakangas [iki.fi] - 2 February 2015 09:09 UTC

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

2b3a8b2 Be more careful to not lose sync in the FE/BE protocol.
src/backend/commands/copy.c | 14 +++
src/backend/libpq/auth.c | 3 +
src/backend/libpq/pqcomm.c | 76 +++++++++++++++-
src/backend/postmaster/postmaster.c | 2 +
src/backend/replication/walsender.c | 35 +++-----
src/backend/storage/lmgr/proc.c | 7 ++
src/backend/tcop/fastpath.c | 29 +-----
src/backend/tcop/postgres.c | 166 +++++++++++++++++++++++------------
src/backend/utils/error/elog.c | 1 +
src/backend/utils/init/globals.c | 1 +
src/include/libpq/libpq.h | 3 +
src/include/miscadmin.h | 13 +++
src/include/tcop/fastpath.h | 1 +
13 files changed, 244 insertions(+), 107 deletions(-)

Upstream: git.postgresql.org

  • Share