Our handling of quoting within replication commands was pretty
sloppy, typically looking like
appendStringInfo(&cmd, " SLOT \"%s\"", options->slotname);
This is fine as long as options->slotname doesn't contain a double
quote mark, but what if it does? In principle this'd allow injection
of harmful options into replication commands, in the probably-unlikely
case that a slot name comes from untrustworthy input. We ought to
clean that up.
Moreover, even the places that were trying to be more careful
generally got it wrong, because they used quoting subroutines
intended for SQL commands rather than something that will work
with the replication-command scanner repl_scanner.l. For example,
several places naively use PQescapeLiteral() to quote option values
for replication commands. If the string contains a backslash,
PQescapeLiteral() will produce E'...' literal syntax, which
repl_scanner.l doesn't recognize. Another near miss was to use
quote_identifier() to quote identifiers. That function won't quote
valid lowercase identifiers unless they match SQL keywords ... but in
this context, replication keywords are what matter. Neither of these
errors seem to risk string injection, but they definitely can cause
syntax errors in replication commands that ought to be valid.
We can clean all this up by using simple quoting logic that just
doubles single or double quotes respectively.
Or at least, we could if repl_scanner.l handled doubled double quotes
in identifiers, but for some reason it doesn't! So the first step in
this fix has to be to fix that. (The fact that we'll later reject
slot names containing double quotes is very far short of justifying
this omission.)
Having done that, this patch runs around and applies correct
quoting in all places that generate replication commands containing
strings coming from outside the immediate context. Probably some
of these places are safe because of restrictions elsewhere, but it
seems best to just quote all the time.
This was originally reported as a security bug, which it could be
if replication slot names or parameters were to originate from
untrustworthy sources. But the security team concluded that that
was a very improbable situation, so we're just going to fix this
as a regular bug.
Reported-by: Team Dhiutsa
Author: Tom Lane <tgl@sss.pgh.pa.us>
Reviewed-by: Ayush Tiwari <ayushtiwari.slg01@gmail.com>
Discussion: https://postgr.es/m/
1648659.
1781287310@sss.pgh.pa.us
Backpatch-through: 14
}
}
+/*
+ * Append a suitably-quoted identifier or string literal to buf.
+ * "quote" should be either a double-quote or single-quote character.
+ *
+ * Caution: this quoting logic is sufficient for identifiers and literals
+ * in the replication grammar, but not always in regular SQL. Specifically,
+ * it'd fail for a string literal if standard_conforming_strings is off.
+ */
+static void
+appendQuotedString(StringInfo buf, const char *str, char quote)
+{
+ appendStringInfoChar(buf, quote);
+ while (*str)
+ {
+ char c = *str++;
+
+ if (c == quote)
+ appendStringInfoChar(buf, c);
+ appendStringInfoChar(buf, c);
+ }
+ appendStringInfoChar(buf, quote);
+}
+
+#define appendQuotedIdentifier(b, s) appendQuotedString(b, s, '"')
+#define appendQuotedLiteral(b, s) appendQuotedString(b, s, '\'')
+
/*
* Check that the specified publications are present on the publisher.
*/
load_file("libpqwalreceiver", false);
initStringInfo(&cmd);
- appendStringInfo(&cmd, "DROP_REPLICATION_SLOT %s WAIT", quote_identifier(slotname));
+ appendStringInfoString(&cmd, "DROP_REPLICATION_SLOT ");
+ appendQuotedIdentifier(&cmd, slotname);
+ appendStringInfoString(&cmd, " WAIT");
PG_TRY();
{
/* Prototypes for private functions */
static PGresult *libpqrcv_PQexec(PGconn *streamConn, const char *query);
static PGresult *libpqrcv_PQgetResult(PGconn *streamConn);
-static char *stringlist_to_identifierstr(PGconn *conn, List *strings);
+static char *stringlist_to_identifierstr(List *strings);
/*
* Module initialization function
return option;
}
+/*
+ * Append a suitably-quoted identifier or string literal to buf.
+ * "quote" should be either a double-quote or single-quote character.
+ *
+ * Caution: this quoting logic is sufficient for identifiers and literals
+ * in the replication grammar, but not always in regular SQL. Specifically,
+ * it'd fail for a string literal if standard_conforming_strings is off.
+ */
+static void
+appendQuotedString(StringInfo buf, const char *str, char quote)
+{
+ appendStringInfoChar(buf, quote);
+ while (*str)
+ {
+ char c = *str++;
+
+ if (c == quote)
+ appendStringInfoChar(buf, c);
+ appendStringInfoChar(buf, c);
+ }
+ appendStringInfoChar(buf, quote);
+}
+
+#define appendQuotedIdentifier(b, s) appendQuotedString(b, s, '"')
+#define appendQuotedLiteral(b, s) appendQuotedString(b, s, '\'')
+
/*
* Start streaming WAL data from given streaming options.
*
/* Build the command. */
appendStringInfoString(&cmd, "START_REPLICATION");
if (options->slotname != NULL)
- appendStringInfo(&cmd, " SLOT \"%s\"",
- options->slotname);
+ {
+ appendStringInfoString(&cmd, " SLOT ");
+ appendQuotedIdentifier(&cmd, options->slotname);
+ }
if (options->logical)
appendStringInfoString(&cmd, " LOGICAL");
{
char *pubnames_str;
List *pubnames;
- char *pubnames_literal;
appendStringInfoString(&cmd, " (");
appendStringInfoString(&cmd, ", two_phase 'on'");
pubnames = options->proto.logical.publication_names;
- pubnames_str = stringlist_to_identifierstr(conn->streamConn, pubnames);
- if (!pubnames_str)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY), /* likely guess */
- errmsg("could not start WAL streaming: %s",
- pchomp(PQerrorMessage(conn->streamConn)))));
- pubnames_literal = PQescapeLiteral(conn->streamConn, pubnames_str,
- strlen(pubnames_str));
- if (!pubnames_literal)
- ereport(ERROR,
- (errcode(ERRCODE_OUT_OF_MEMORY), /* likely guess */
- errmsg("could not start WAL streaming: %s",
- pchomp(PQerrorMessage(conn->streamConn)))));
- appendStringInfo(&cmd, ", publication_names %s", pubnames_literal);
- PQfreemem(pubnames_literal);
+ pubnames_str = stringlist_to_identifierstr(pubnames);
+ appendStringInfoString(&cmd, ", publication_names ");
+ appendQuotedLiteral(&cmd, pubnames_str);
pfree(pubnames_str);
if (options->proto.logical.binary &&
initStringInfo(&cmd);
- appendStringInfo(&cmd, "CREATE_REPLICATION_SLOT \"%s\"", slotname);
+ appendStringInfoString(&cmd, "CREATE_REPLICATION_SLOT ");
+ appendQuotedIdentifier(&cmd, slotname);
if (temporary)
appendStringInfoString(&cmd, " TEMPORARY");
*
* This is essentially the reverse of SplitIdentifierString.
*
- * The caller should free the result.
+ * The caller should pfree the result.
*/
static char *
-stringlist_to_identifierstr(PGconn *conn, List *strings)
+stringlist_to_identifierstr(List *strings)
{
ListCell *lc;
StringInfoData res;
foreach(lc, strings)
{
char *val = strVal(lfirst(lc));
- char *val_escaped;
if (first)
first = false;
else
appendStringInfoChar(&res, ',');
-
- val_escaped = PQescapeIdentifier(conn, val, strlen(val));
- if (!val_escaped)
- {
- free(res.data);
- return NULL;
- }
- appendStringInfoString(&res, val_escaped);
- PQfreemem(val_escaped);
+ appendQuotedIdentifier(&res, val);
}
return res.data;
return IDENT;
}
+<xd>{xddouble} {
+ addlitchar('"');
+ }
+
<xd>{xdinside} {
addlit(yytext, yyleng);
}
/* Initiate the replication stream at specified location */
query = createPQExpBuffer();
- appendPQExpBuffer(query, "START_REPLICATION SLOT \"%s\" LOGICAL %X/%X",
- replication_slot, LSN_FORMAT_ARGS(startpos));
+ appendPQExpBufferStr(query, "START_REPLICATION SLOT ");
+ AppendQuotedIdentifier(query, replication_slot);
+ appendPQExpBuffer(query, " LOGICAL %X/%X", LSN_FORMAT_ARGS(startpos));
/* print options if there are any */
if (noptions)
appendPQExpBufferStr(query, ", ");
/* write option name */
- appendPQExpBuffer(query, "\"%s\"", options[(i * 2)]);
+ AppendQuotedIdentifier(query, options[i * 2]);
/* write option value if specified */
- if (options[(i * 2) + 1] != NULL)
- appendPQExpBuffer(query, " '%s'", options[(i * 2) + 1]);
+ if (options[i * 2 + 1] != NULL)
+ {
+ appendPQExpBufferChar(query, ' ');
+ AppendQuotedLiteral(query, options[i * 2 + 1]);
+ }
}
if (noptions)
bool
ReceiveXlogStream(PGconn *conn, StreamCtl *stream)
{
- char query[128];
- char slotcmd[128];
+ PQExpBuffer query;
PGresult *res;
XLogRecPtr stoppos;
if (stream->replication_slot != NULL)
{
reportFlushPosition = true;
- sprintf(slotcmd, "SLOT \"%s\" ", stream->replication_slot);
}
else
{
reportFlushPosition = true;
else
reportFlushPosition = false;
- slotcmd[0] = 0;
}
if (stream->sysidentifier != NULL)
*/
if (!existsTimeLineHistoryFile(stream))
{
- snprintf(query, sizeof(query), "TIMELINE_HISTORY %u", stream->timeline);
- res = PQexec(conn, query);
+ query = createPQExpBuffer();
+ appendPQExpBuffer(query, "TIMELINE_HISTORY %u", stream->timeline);
+ res = PQexec(conn, query->data);
+ destroyPQExpBuffer(query);
if (PQresultStatus(res) != PGRES_TUPLES_OK)
{
/* FIXME: we might send it ok, but get an error */
return true;
/* Initiate the replication stream at specified location */
- snprintf(query, sizeof(query), "START_REPLICATION %s%X/%X TIMELINE %u",
- slotcmd,
- LSN_FORMAT_ARGS(stream->startpos),
- stream->timeline);
- res = PQexec(conn, query);
+ query = createPQExpBuffer();
+ appendPQExpBufferStr(query, "START_REPLICATION");
+ if (stream->replication_slot != NULL)
+ {
+ appendPQExpBufferStr(query, " SLOT ");
+ AppendQuotedIdentifier(query, stream->replication_slot);
+ }
+ appendPQExpBuffer(query, " %X/%X TIMELINE %u",
+ LSN_FORMAT_ARGS(stream->startpos),
+ stream->timeline);
+ res = PQexec(conn, query->data);
+ destroyPQExpBuffer(query);
if (PQresultStatus(res) != PGRES_COPY_BOTH)
{
pg_log_error("could not send replication command \"%s\": %s",
*restart_tli = tli_loc;
query = createPQExpBuffer();
- appendPQExpBuffer(query, "READ_REPLICATION_SLOT %s", slot_name);
+ appendPQExpBufferStr(query, "READ_REPLICATION_SLOT ");
+ AppendQuotedIdentifier(query, slot_name);
res = PQexec(conn, query->data);
destroyPQExpBuffer(query);
Assert(slot_name != NULL);
/* Build base portion of query */
- appendPQExpBuffer(query, "CREATE_REPLICATION_SLOT \"%s\"", slot_name);
+ appendPQExpBufferStr(query, "CREATE_REPLICATION_SLOT ");
+ AppendQuotedIdentifier(query, slot_name);
if (is_temporary)
appendPQExpBufferStr(query, " TEMPORARY");
if (is_physical)
appendPQExpBufferStr(query, " PHYSICAL");
else
- appendPQExpBuffer(query, " LOGICAL \"%s\"", plugin);
+ {
+ appendPQExpBufferStr(query, " LOGICAL ");
+ AppendQuotedIdentifier(query, plugin);
+ }
/* Add any requested options */
if (use_new_option_syntax)
query = createPQExpBuffer();
/* Build query */
- appendPQExpBuffer(query, "DROP_REPLICATION_SLOT \"%s\"",
- slot_name);
+ appendPQExpBufferStr(query, "DROP_REPLICATION_SLOT ");
+ AppendQuotedIdentifier(query, slot_name);
res = PQexec(conn, query->data);
if (PQresultStatus(res) != PGRES_COMMAND_OK)
{
return true;
}
+/*
+ * Append a suitably-quoted identifier or string literal to buf.
+ * "quote" should be either a double-quote or single-quote character.
+ *
+ * Caution: this quoting logic is sufficient for identifiers and literals
+ * in the replication grammar, but not always in regular SQL. Specifically,
+ * it'd fail for a string literal if standard_conforming_strings is off.
+ */
+void
+AppendQuotedString(PQExpBuffer buf, const char *str, char quote)
+{
+ appendPQExpBufferChar(buf, quote);
+ while (*str)
+ {
+ char c = *str++;
+
+ if (c == quote)
+ appendPQExpBufferChar(buf, c);
+ appendPQExpBufferChar(buf, c);
+ }
+ appendPQExpBufferChar(buf, quote);
+}
+
/*
* Append a "plain" option - one with no value - to a server command that
* is being constructed.
* write things like SOME_COMMAND OPTION1 OPTION2 'opt2value' OPTION3 42. The
* new syntax uses a comma-separated list surrounded by parentheses, so the
* equivalent is SOME_COMMAND (OPTION1, OPTION2 'optvalue', OPTION3 42).
+ *
+ * Note: we assume option names do not require quotes. Do not use this
+ * with option names coming from outside sources.
*/
void
AppendPlainCommandOption(PQExpBuffer buf, bool use_new_option_syntax,
- char *option_name)
+ const char *option_name)
{
if (buf->len > 0 && buf->data[buf->len - 1] != '(')
{
*/
void
AppendStringCommandOption(PQExpBuffer buf, bool use_new_option_syntax,
- char *option_name, char *option_value)
+ const char *option_name, const char *option_value)
{
AppendPlainCommandOption(buf, use_new_option_syntax, option_name);
if (option_value != NULL)
{
- size_t length = strlen(option_value);
- char *escaped_value = palloc(1 + 2 * length);
-
- PQescapeStringConn(conn, escaped_value, option_value, length, NULL);
- appendPQExpBuffer(buf, " '%s'", escaped_value);
- pfree(escaped_value);
+ appendPQExpBufferChar(buf, ' ');
+ AppendQuotedLiteral(buf, option_value);
}
}
/*
- * Append an option with an associated integer value to a server command
+ * Append an option with an associated integer value to a server command that
* is being constructed.
*
* See comments for AppendPlainCommandOption, above.
*/
void
AppendIntegerCommandOption(PQExpBuffer buf, bool use_new_option_syntax,
- char *option_name, int32 option_value)
+ const char *option_name, int32 option_value)
{
AppendPlainCommandOption(buf, use_new_option_syntax, option_name);
XLogRecPtr *startpos,
char **db_name);
+extern void AppendQuotedString(PQExpBuffer buf, const char *str, char quote);
+#define AppendQuotedIdentifier(b, s) AppendQuotedString(b, s, '"')
+#define AppendQuotedLiteral(b, s) AppendQuotedString(b, s, '\'')
extern void AppendPlainCommandOption(PQExpBuffer buf,
bool use_new_option_syntax,
- char *option_value);
+ const char *option_name);
extern void AppendStringCommandOption(PQExpBuffer buf,
bool use_new_option_syntax,
- char *option_name, char *option_value);
+ const char *option_name,
+ const char *option_value);
extern void AppendIntegerCommandOption(PQExpBuffer buf,
bool use_new_option_syntax,
- char *option_name, int32 option_value);
+ const char *option_name,
+ int32 option_value);
extern bool GetSlotInformation(PGconn *conn, const char *slot_name,
XLogRecPtr *restart_lsn,
projects / thirdparty / postgresql.git / commitdiff
