From b3eb4ea5d8c35ba9ec890b0f51d04c0b62d53a03 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 21 May 2006 20:10:42 +0000 Subject: [PATCH] Add a new GUC parameter backslash_quote, which determines whether the SQL parser will allow "\'" to be used to represent a literal quote mark. The "\'" representation has been deprecated for some time in favor of the SQL-standard representation "''" (two single quote marks), but it has been used often enough that just disallowing it immediately won't do. Hence backslash_quote allows the settings "on", "off", and "safe_encoding", the last meaning to allow "\'" only if client_encoding is a valid server encoding. That is now the default, and the reason is that in encodings such as SJIS that allow 0x5c (ASCII backslash) to be the last byte of a multibyte character, accepting "\'" allows SQL-injection attacks as per CVE-2006-2314 (further details will be published after release). The "on" setting is available for backward compatibility, but it must not be used with clients that are exposed to untrusted input. Thanks to Akio Ishida and Yasuo Ohgaki for identifying this security issue. --- doc/src/sgml/config.sgml | 38 +++++++++++++++++- src/backend/parser/scan.l | 14 ++++++- src/backend/utils/misc/guc.c | 39 ++++++++++++++++++- src/backend/utils/misc/postgresql.conf.sample | 1 + src/include/parser/gramparse.h | 10 ++++- 5 files changed, 98 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index a2e6d46ca6..8ab95a608c 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -1,4 +1,4 @@ - + Server Configuration @@ -3695,6 +3695,42 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' + + backslash_quote (string) + stringsbackslash quotes + + backslash_quote configuration parameter + + + + This controls whether a quote mark can be represented by + \' in a string literal. The preferred, SQL-standard way + to represent a quote mark is by doubling it ('') but + PostgreSQL has historically also accepted + \'. However, use of \' creates security risks + because in some client character set encodings, there are multibyte + characters in which the last byte is numerically equivalent to ASCII + \. If client-side code does escaping incorrectly then a + SQL-injection attack is possible. This risk can be prevented by + making the server reject queries in which a quote mark appears to be + escaped by a backslash. + The allowed values of backslash_quote are + on (allow \' always), + off (reject always), and + safe_encoding (allow only if client encoding does not + allow ASCII \ within a multibyte character). + safe_encoding is the default setting. + + + + Note that in a standard-conforming string literal, \ just + means \ anyway. This parameter affects the handling of + non-standard-conforming literals, including + escape string syntax (E'...'). + + + + default_with_oids (boolean) diff --git a/src/backend/parser/scan.l b/src/backend/parser/scan.l index 78a888e02a..13e9604e38 100644 --- a/src/backend/parser/scan.l +++ b/src/backend/parser/scan.l @@ -24,7 +24,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/scan.l,v 1.134 2006/05/11 19:15:35 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/parser/scan.l,v 1.135 2006/05/21 20:10:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -55,6 +55,7 @@ static char *dolqstart; /* current $foo$ quote start string */ * But we shall have to live with it as a short-term thing until the switch * to SQL-standard string syntax is complete. */ +BackslashQuoteType backslash_quote = BACKSLASH_QUOTE_SAFE_ENCODING; bool escape_string_warning = true; bool standard_conforming_strings = false; @@ -452,6 +453,17 @@ other . addlit(yytext, yyleng); } {xeescape} { + if (yytext[1] == '\'') + { + if (backslash_quote == BACKSLASH_QUOTE_OFF || + (backslash_quote == BACKSLASH_QUOTE_SAFE_ENCODING && + PG_ENCODING_IS_CLIENT_ONLY(pg_get_client_encoding()))) + ereport(ERROR, + (errcode(ERRCODE_NONSTANDARD_USE_OF_ESCAPE_CHARACTER), + errmsg("unsafe use of \\' in a string literal"), + errhint("Use '' to write quotes in strings. \\' is insecure in client-only encodings."), + lexer_errposition())); + } check_string_escape_warning(yytext[1]); addlitchar(unescape_single_char(yytext[1])); } diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 87df3fdbac..1ff2e0c831 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -10,7 +10,7 @@ * Written by Peter Eisentraut . * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.319 2006/05/11 19:15:35 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.320 2006/05/21 20:10:42 tgl Exp $ * *-------------------------------------------------------------------- */ @@ -138,6 +138,7 @@ static bool assign_stage_log_stats(bool newval, bool doit, GucSource source); static bool assign_log_stats(bool newval, bool doit, GucSource source); static bool assign_transaction_read_only(bool newval, bool doit, GucSource source); static const char *assign_canonical_path(const char *newval, bool doit, GucSource source); +static const char *assign_backslash_quote(const char *newval, bool doit, GucSource source); static bool assign_tcp_keepalives_idle(int newval, bool doit, GucSource source); static bool assign_tcp_keepalives_interval(int newval, bool doit, GucSource source); @@ -210,6 +211,7 @@ static char *syslog_ident_str; static bool phony_autocommit; static bool session_auth_is_superuser; static double phony_random_seed; +static char *backslash_quote_string; static char *client_encoding_string; static char *datestyle_string; static char *default_iso_level_string; @@ -1716,6 +1718,15 @@ static struct config_string ConfigureNamesString[] = "", NULL, NULL }, + { + {"backslash_quote", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS, + gettext_noop("Sets whether \"\\'\" is allowed in string literals."), + gettext_noop("Valid values are ON, OFF, and SAFE_ENCODING.") + }, + &backslash_quote_string, + "safe_encoding", assign_backslash_quote, NULL + }, + { {"client_encoding", PGC_USERSET, CLIENT_CONN_LOCALE, gettext_noop("Sets the client's character set encoding."), @@ -6056,6 +6067,32 @@ assign_canonical_path(const char *newval, bool doit, GucSource source) return newval; } +static const char * +assign_backslash_quote(const char *newval, bool doit, GucSource source) +{ + BackslashQuoteType bq; + bool bqbool; + + /* + * Although only "on", "off", and "safe_encoding" are documented, + * we use parse_bool so we can accept all the likely variants of + * "on" and "off". + */ + if (pg_strcasecmp(newval, "safe_encoding") == 0) + bq = BACKSLASH_QUOTE_SAFE_ENCODING; + else if (parse_bool(newval, &bqbool)) + { + bq = bqbool ? BACKSLASH_QUOTE_ON : BACKSLASH_QUOTE_OFF; + } + else + return NULL; /* reject */ + + if (doit) + backslash_quote = bq; + + return newval; +} + static bool assign_tcp_keepalives_idle(int newval, bool doit, GucSource source) { diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index dac59378a0..d7f32256d1 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -414,6 +414,7 @@ #add_missing_from = off #array_nulls = on +#backslash_quote = safe_encoding # on, off, or safe_encoding #default_with_oids = off #escape_string_warning = on #standard_conforming_strings = off diff --git a/src/include/parser/gramparse.h b/src/include/parser/gramparse.h index 8430ad8c8e..38f4b26f94 100644 --- a/src/include/parser/gramparse.h +++ b/src/include/parser/gramparse.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/parser/gramparse.h,v 1.35 2006/05/11 19:15:35 tgl Exp $ + * $PostgreSQL: pgsql/src/include/parser/gramparse.h,v 1.36 2006/05/21 20:10:42 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -27,7 +27,15 @@ */ #define YYLTYPE int +typedef enum +{ + BACKSLASH_QUOTE_OFF, + BACKSLASH_QUOTE_ON, + BACKSLASH_QUOTE_SAFE_ENCODING +} BackslashQuoteType; + /* GUC variables in scan.l (every one of these is a bad idea :-() */ +extern BackslashQuoteType backslash_quote; extern bool escape_string_warning; extern bool standard_conforming_strings;