From 3f63787cbfe0f1e837c92cd8ac35fd7ab811c18b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 4 Sep 2002 23:31:35 +0000 Subject: [PATCH] Guard against send-lots-and-lots-of-data DoS attack from unauthenticated users, by limiting the length of string we will accept for a password. Patch by Serguei Mokhov, some editorializing by Tom Lane. --- src/backend/libpq/auth.c | 9 ++++----- src/backend/libpq/be-secure.c | 4 ++-- src/backend/libpq/pqcomm.c | 21 ++++++++++++++++----- src/backend/libpq/pqformat.c | 18 ++++++++++-------- src/include/libpq/libpq.h | 4 ++-- src/include/libpq/pqformat.h | 6 ++++-- 6 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 7be74c58a7..d036a22f6e 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/libpq/auth.c,v 1.90 2002/09/04 20:31:18 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/libpq/auth.c,v 1.91 2002/09/04 23:31:34 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -563,12 +563,11 @@ pam_passwd_conv_proc(int num_msg, const struct pam_message ** msg, struct pam_re { sendAuthRequest(pam_port_cludge, AUTH_REQ_PASSWORD); if (pq_eof() == EOF || pq_getint(&len, 4) == EOF) - { return PAM_CONV_ERR; /* client didn't want to send password */ - } initStringInfo(&buf); - pq_getstr(&buf); + if (pq_getstr_bounded(&buf, 1000) == EOF) + return PAM_CONV_ERR; /* EOF while reading password */ /* Do not echo failed password to logs, for security. */ elog(DEBUG5, "received PAM packet"); @@ -707,7 +706,7 @@ recv_and_check_password_packet(Port *port) return STATUS_EOF; /* client didn't want to send password */ initStringInfo(&buf); - if (pq_getstr(&buf) == EOF) /* receive password */ + if (pq_getstr_bounded(&buf, 1000) == EOF) /* receive password */ { pfree(buf.data); return STATUS_EOF; diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index d7dca96528..6baf568eea 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -1,6 +1,6 @@ /*------------------------------------------------------------------------- * - * be-connect.c + * be-secure.c * functions related to setting up a secure connection to the frontend. * Secure connections are expected to provide confidentiality, * message integrity and endpoint authentication. @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $Header: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v 1.13 2002/09/04 20:31:19 momjian Exp $ + * $Header: /cvsroot/pgsql/src/backend/libpq/be-secure.c,v 1.14 2002/09/04 23:31:34 tgl Exp $ * * Since the server static private key ($DataDir/server.key) * will normally be stored unencrypted so that the database diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c index c0d832bd3d..62e8bd44cd 100644 --- a/src/backend/libpq/pqcomm.c +++ b/src/backend/libpq/pqcomm.c @@ -29,7 +29,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: pqcomm.c,v 1.140 2002/09/04 20:31:19 momjian Exp $ + * $Id: pqcomm.c,v 1.141 2002/09/04 23:31:34 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -555,16 +555,24 @@ pq_getbytes(char *s, size_t len) * The return value is placed in an expansible StringInfo. * Note that space allocation comes from the current memory context! * + * If maxlen is not zero, it is an upper limit on the length of the + * string we are willing to accept. We abort the connection (by + * returning EOF) if client tries to send more than that. Note that + * since we test maxlen in the outer per-bufferload loop, the limit + * is fuzzy: we might accept up to PQ_BUFFER_SIZE more bytes than + * specified. This is fine for the intended purpose, which is just + * to prevent DoS attacks from not-yet-authenticated clients. + * * NOTE: this routine does not do any character set conversion, * even though it is presumably useful only for text, because * no code in this module should depend on the encoding. - * See pq_getstr in pqformat.c for that. + * See pq_getstr_bounded in pqformat.c for that. * * returns 0 if OK, EOF if trouble * -------------------------------- */ int -pq_getstring(StringInfo s) +pq_getstring(StringInfo s, int maxlen) { int i; @@ -572,7 +580,7 @@ pq_getstring(StringInfo s) s->len = 0; s->data[0] = '\0'; - /* Read until we get the terminating '\0' */ + /* Read until we get the terminating '\0' or overrun maxlen */ for (;;) { while (PqRecvPointer >= PqRecvLength) @@ -594,10 +602,13 @@ pq_getstring(StringInfo s) } /* If we're here we haven't got the \0 in the buffer yet. */ - appendBinaryStringInfo(s, PqRecvBuffer + PqRecvPointer, PqRecvLength - PqRecvPointer); PqRecvPointer = PqRecvLength; + + /* If maxlen is specified, check for overlength input. */ + if (maxlen > 0 && s->len > maxlen) + return EOF; } } diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c index 278835f209..5f5acb4443 100644 --- a/src/backend/libpq/pqformat.c +++ b/src/backend/libpq/pqformat.c @@ -16,7 +16,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: pqformat.c,v 1.24 2002/09/03 21:45:42 petere Exp $ + * $Id: pqformat.c,v 1.25 2002/09/04 23:31:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -38,10 +38,10 @@ * pq_puttextmessage - generate a character set-converted message in one step * * Message input: - * pq_getint - get an integer from connection - * pq_getstr - get a null terminated string from connection - * pq_getstr performs character set conversion on the collected string. - * Use the raw pqcomm.c routines pq_getstring or pq_getbytes + * pq_getint - get an integer from connection + * pq_getstr_bounded - get a null terminated string from connection + * pq_getstr_bounded performs character set conversion on the collected + * string. Use the raw pqcomm.c routines pq_getstring or pq_getbytes * to fetch data without conversion. */ @@ -247,21 +247,23 @@ pq_getint(int *result, int b) } /* -------------------------------- - * pq_getstr - get a null terminated string from connection + * pq_getstr_bounded - get a null terminated string from connection * * The return value is placed in an expansible StringInfo. * Note that space allocation comes from the current memory context! * + * The maxlen parameter is interpreted as per pq_getstring. + * * returns 0 if OK, EOF if trouble * -------------------------------- */ int -pq_getstr(StringInfo s) +pq_getstr_bounded(StringInfo s, int maxlen) { int result; char *p; - result = pq_getstring(s); + result = pq_getstring(s, maxlen); p = (char *) pg_client_to_server((unsigned char *) s->data, s->len); if (p != s->data) /* actual conversion has been done? */ diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h index 915b3222a0..5e4db4d243 100644 --- a/src/include/libpq/libpq.h +++ b/src/include/libpq/libpq.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: libpq.h,v 1.51 2002/06/20 20:29:49 momjian Exp $ + * $Id: libpq.h,v 1.52 2002/09/04 23:31:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -50,7 +50,7 @@ extern int StreamConnection(int server_fd, Port *port); extern void StreamClose(int sock); extern void pq_init(void); extern int pq_getbytes(char *s, size_t len); -extern int pq_getstring(StringInfo s); +extern int pq_getstring(StringInfo s, int maxlen); extern int pq_getbyte(void); extern int pq_peekbyte(void); extern int pq_putbytes(const char *s, size_t len); diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h index b94cf7a151..829727c38f 100644 --- a/src/include/libpq/pqformat.h +++ b/src/include/libpq/pqformat.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $Id: pqformat.h,v 1.12 2002/06/20 20:29:49 momjian Exp $ + * $Id: pqformat.h,v 1.13 2002/09/04 23:31:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -27,6 +27,8 @@ extern void pq_endmessage(StringInfo buf); extern int pq_puttextmessage(char msgtype, const char *str); extern int pq_getint(int *result, int b); -extern int pq_getstr(StringInfo s); +extern int pq_getstr_bounded(StringInfo s, int maxlen); + +#define pq_getstr(s) pq_getstr_bounded(s, 0) #endif /* PQFORMAT_H */