Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit95a6855

Browse files
committed
Obstruct shell, SQL, and conninfo injection via database and role names.
Due to simplistic quoting and confusion of database names with conninfostrings, roles with the CREATEDB or CREATEROLE option could escalate tosuperuser privileges when a superuser next ran certain maintenancecommands. The new coding rule for PQconnectdbParams() calls, documentedat conninfo_array_parse(), is to pass expand_dbname=true and wrapliteral database names in a trivial connection string. Escapezero-length values in appendConnStrVal(). Back-patch to 9.1 (allsupported versions).Nathan Bossart, Michael Paquier, and Noah Misch. Reviewed by PeterEisentraut. Reported by Nathan Bossart.Security:CVE-2016-5424
1 parentc1b048f commit95a6855

File tree

21 files changed

+665
-202
lines changed

21 files changed

+665
-202
lines changed

‎contrib/pg_upgrade/dump.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,17 +46,28 @@ generate_old_dump(void)
4646
charsql_file_name[MAXPGPATH],
4747
log_file_name[MAXPGPATH];
4848
DbInfo*old_db=&old_cluster.dbarr.dbs[dbnum];
49+
PQExpBufferDataconnstr,
50+
escaped_connstr;
51+
52+
initPQExpBuffer(&connstr);
53+
appendPQExpBuffer(&connstr,"dbname=");
54+
appendConnStrVal(&connstr,old_db->db_name);
55+
initPQExpBuffer(&escaped_connstr);
56+
appendShellString(&escaped_connstr,connstr.data);
57+
termPQExpBuffer(&connstr);
4958

5059
pg_log(PG_STATUS,"%s",old_db->db_name);
5160
snprintf(sql_file_name,sizeof(sql_file_name),DB_DUMP_FILE_MASK,old_db->db_oid);
5261
snprintf(log_file_name,sizeof(log_file_name),DB_DUMP_LOG_FILE_MASK,old_db->db_oid);
5362

5463
parallel_exec_prog(log_file_name,NULL,
5564
"\"%s/pg_dump\" %s --schema-only --quote-all-identifiers "
56-
"--binary-upgrade --format=custom %s --file=\"%s\"\"%s\"",
65+
"--binary-upgrade --format=custom %s --file=\"%s\"%s",
5766
new_cluster.bindir,cluster_conn_opts(&old_cluster),
5867
log_opts.verbose ?"--verbose" :"",
59-
sql_file_name,old_db->db_name);
68+
sql_file_name,escaped_connstr.data);
69+
70+
termPQExpBuffer(&escaped_connstr);
6071
}
6172

6273
/* reap all children */

‎contrib/pg_upgrade/pg_upgrade.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,15 @@ create_new_objects(void)
498498
charsql_file_name[MAXPGPATH],
499499
log_file_name[MAXPGPATH];
500500
DbInfo*old_db=&old_cluster.dbarr.dbs[dbnum];
501+
PQExpBufferDataconnstr,
502+
escaped_connstr;
503+
504+
initPQExpBuffer(&connstr);
505+
appendPQExpBuffer(&connstr,"dbname=");
506+
appendConnStrVal(&connstr,old_db->db_name);
507+
initPQExpBuffer(&escaped_connstr);
508+
appendShellString(&escaped_connstr,connstr.data);
509+
termPQExpBuffer(&connstr);
501510

502511
pg_log(PG_STATUS,"%s",old_db->db_name);
503512
snprintf(sql_file_name,sizeof(sql_file_name),DB_DUMP_FILE_MASK,old_db->db_oid);
@@ -509,11 +518,13 @@ create_new_objects(void)
509518
*/
510519
parallel_exec_prog(log_file_name,
511520
NULL,
512-
"\"%s/pg_restore\" %s --exit-on-error --verbose --dbname\"%s\" \"%s\"",
521+
"\"%s/pg_restore\" %s --exit-on-error --verbose --dbname%s \"%s\"",
513522
new_cluster.bindir,
514523
cluster_conn_opts(&new_cluster),
515-
old_db->db_name,
524+
escaped_connstr.data,
516525
sql_file_name);
526+
527+
termPQExpBuffer(&escaped_connstr);
517528
}
518529

519530
/* reap all children */

‎contrib/pg_upgrade/pg_upgrade.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include<sys/time.h>
1212

1313
#include"libpq-fe.h"
14+
#include"pqexpbuffer.h"
1415

1516
/* Use port in the private/dynamic port number range */
1617
#defineDEF_PGUPORT50432
@@ -440,6 +441,9 @@ voidcheck_pghost_envvar(void);
440441
/* util.c */
441442

442443
char*quote_identifier(constchar*s);
444+
externvoidappendShellString(PQExpBufferbuf,constchar*str);
445+
externvoidappendConnStrVal(PQExpBufferbuf,constchar*str);
446+
externvoidappendPsqlMetaConnect(PQExpBufferbuf,constchar*dbname);
443447
intget_user_info(char**user_name);
444448
voidcheck_ok(void);
445449
void

‎contrib/pg_upgrade/server.c

Lines changed: 31 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -51,18 +51,25 @@ connectToServer(ClusterInfo *cluster, const char *db_name)
5151
staticPGconn*
5252
get_db_conn(ClusterInfo*cluster,constchar*db_name)
5353
{
54-
charconn_opts[2*NAMEDATALEN+MAXPGPATH+100];
54+
PQExpBufferDataconn_opts;
55+
PGconn*conn;
5556

57+
/* Build connection string with proper quoting */
58+
initPQExpBuffer(&conn_opts);
59+
appendPQExpBufferStr(&conn_opts,"dbname=");
60+
appendConnStrVal(&conn_opts,db_name);
61+
appendPQExpBufferStr(&conn_opts," user=");
62+
appendConnStrVal(&conn_opts,os_info.user);
63+
appendPQExpBuffer(&conn_opts," port=%d",cluster->port);
5664
if (cluster->sockdir)
57-
snprintf(conn_opts,sizeof(conn_opts),
58-
"dbname = '%s' user = '%s' host = '%s' port = %d",
59-
db_name,os_info.user,cluster->sockdir,cluster->port);
60-
else
61-
snprintf(conn_opts,sizeof(conn_opts),
62-
"dbname = '%s' user = '%s' port = %d",
63-
db_name,os_info.user,cluster->port);
65+
{
66+
appendPQExpBufferStr(&conn_opts," host=");
67+
appendConnStrVal(&conn_opts,cluster->sockdir);
68+
}
6469

65-
returnPQconnectdb(conn_opts);
70+
conn=PQconnectdb(conn_opts.data);
71+
termPQExpBuffer(&conn_opts);
72+
returnconn;
6673
}
6774

6875

@@ -74,23 +81,28 @@ get_db_conn(ClusterInfo *cluster, const char *db_name)
7481
* sets, but the utilities we need aren't very consistent about the treatment
7582
* of database name options, so we leave that out.
7683
*
77-
*Note resultisin static storage, so use it right away.
84+
*Resultisvalid until the next call to this function.
7885
*/
7986
char*
8087
cluster_conn_opts(ClusterInfo*cluster)
8188
{
82-
staticcharconn_opts[MAXPGPATH+NAMEDATALEN+100];
89+
staticPQExpBufferbuf;
8390

84-
if (cluster->sockdir)
85-
snprintf(conn_opts,sizeof(conn_opts),
86-
"--host \"%s\" --port %d --username \"%s\"",
87-
cluster->sockdir,cluster->port,os_info.user);
91+
if (buf==NULL)
92+
buf=createPQExpBuffer();
8893
else
89-
snprintf(conn_opts,sizeof(conn_opts),
90-
"--port %d --username \"%s\"",
91-
cluster->port,os_info.user);
94+
resetPQExpBuffer(buf);
95+
96+
if (cluster->sockdir)
97+
{
98+
appendPQExpBufferStr(buf,"--host ");
99+
appendShellString(buf,cluster->sockdir);
100+
appendPQExpBufferChar(buf,' ');
101+
}
102+
appendPQExpBuffer(buf,"--port %d --username ",cluster->port);
103+
appendShellString(buf,os_info.user);
92104

93-
returnconn_opts;
105+
returnbuf->data;
94106
}
95107

96108

‎contrib/pg_upgrade/test.sh

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,20 @@ set -x
155155

156156
standard_initdb"$oldbindir"/initdb
157157
$oldbindir/pg_ctl start -l"$logdir/postmaster1.log" -o"$POSTMASTER_OPTS" -w
158+
159+
# Create databases with names covering the ASCII bytes other than NUL, BEL,
160+
# LF, or CR. BEL would ring the terminal bell in the course of this test, and
161+
# it is not otherwise a special case. PostgreSQL doesn't support the rest.
162+
dbname1=`awk'BEGIN { for (i= 1; i < 46; i++)
163+
if (i != 7 && i != 10 && i != 13) printf "%c", i }'</dev/null`
164+
# Exercise backslashes adjacent to double quotes, a Windows special case.
165+
dbname1='\"\'$dbname1'\\"\\\'
166+
dbname2=`awk'BEGIN { for (i = 46; i < 91; i++) printf "%c", i }'</dev/null`
167+
dbname3=`awk'BEGIN { for (i = 91; i < 128; i++) printf "%c", i }'</dev/null`
168+
createdb"$dbname1"|| createdb_status=$?
169+
createdb"$dbname2"|| createdb_status=$?
170+
createdb"$dbname3"|| createdb_status=$?
171+
158172
if"$MAKE" -C"$oldsrc" installcheck;then
159173
pg_dumpall -f"$temp_root"/dump1.sql|| pg_dumpall1_status=$?
160174
if ["$newsrc"!="$oldsrc" ];then
@@ -180,6 +194,9 @@ else
180194
make_installcheck_status=$?
181195
fi
182196
$oldbindir/pg_ctl -m fast stop
197+
if [-n"$createdb_status" ];then
198+
exit 1
199+
fi
183200
if [-n"$make_installcheck_status" ];then
184201
exit 1
185202
fi

‎contrib/pg_upgrade/util.c

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,210 @@ quote_identifier(const char *s)
180180
}
181181

182182

183+
/*
184+
* Append the given string to the shell command being built in the buffer,
185+
* with suitable shell-style quoting to create exactly one argument.
186+
*
187+
* Forbid LF or CR characters, which have scant practical use beyond designing
188+
* security breaches. The Windows command shell is unusable as a conduit for
189+
* arguments containing LF or CR characters. A future major release should
190+
* reject those characters in CREATE ROLE and CREATE DATABASE, because use
191+
* there eventually leads to errors here.
192+
*/
193+
void
194+
appendShellString(PQExpBufferbuf,constchar*str)
195+
{
196+
constchar*p;
197+
198+
#ifndefWIN32
199+
appendPQExpBufferChar(buf,'\'');
200+
for (p=str;*p;p++)
201+
{
202+
if (*p=='\n'||*p=='\r')
203+
{
204+
fprintf(stderr,
205+
_("shell command argument contains a newline or carriage return: \"%s\"\n"),
206+
str);
207+
exit(EXIT_FAILURE);
208+
}
209+
210+
if (*p=='\'')
211+
appendPQExpBufferStr(buf,"'\"'\"'");
212+
else
213+
appendPQExpBufferChar(buf,*p);
214+
}
215+
appendPQExpBufferChar(buf,'\'');
216+
#else/* WIN32 */
217+
intbackslash_run_length=0;
218+
219+
/*
220+
* A Windows system() argument experiences two layers of interpretation.
221+
* First, cmd.exe interprets the string. Its behavior is undocumented,
222+
* but a caret escapes any byte except LF or CR that would otherwise have
223+
* special meaning. Handling of a caret before LF or CR differs between
224+
* "cmd.exe /c" and other modes, and it is unusable here.
225+
*
226+
* Second, the new process parses its command line to construct argv (see
227+
* https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats
228+
* backslash-double quote sequences specially.
229+
*/
230+
appendPQExpBufferStr(buf,"^\"");
231+
for (p=str;*p;p++)
232+
{
233+
if (*p=='\n'||*p=='\r')
234+
{
235+
fprintf(stderr,
236+
_("shell command argument contains a newline or carriage return: \"%s\"\n"),
237+
str);
238+
exit(EXIT_FAILURE);
239+
}
240+
241+
/* Change N backslashes before a double quote to 2N+1 backslashes. */
242+
if (*p=='"')
243+
{
244+
while (backslash_run_length)
245+
{
246+
appendPQExpBufferStr(buf,"^\\");
247+
backslash_run_length--;
248+
}
249+
appendPQExpBufferStr(buf,"^\\");
250+
}
251+
elseif (*p=='\\')
252+
backslash_run_length++;
253+
else
254+
backslash_run_length=0;
255+
256+
/*
257+
* Decline to caret-escape the most mundane characters, to ease
258+
* debugging and lest we approach the command length limit.
259+
*/
260+
if (!((*p >='a'&&*p <='z')||
261+
(*p >='A'&&*p <='Z')||
262+
(*p >='0'&&*p <='9')))
263+
appendPQExpBufferChar(buf,'^');
264+
appendPQExpBufferChar(buf,*p);
265+
}
266+
267+
/*
268+
* Change N backslashes at end of argument to 2N backslashes, because they
269+
* precede the double quote that terminates the argument.
270+
*/
271+
while (backslash_run_length)
272+
{
273+
appendPQExpBufferStr(buf,"^\\");
274+
backslash_run_length--;
275+
}
276+
appendPQExpBufferStr(buf,"^\"");
277+
#endif/* WIN32 */
278+
}
279+
280+
281+
/*
282+
* Append the given string to the buffer, with suitable quoting for passing
283+
* the string as a value, in a keyword/pair value in a libpq connection
284+
* string
285+
*/
286+
void
287+
appendConnStrVal(PQExpBufferbuf,constchar*str)
288+
{
289+
constchar*s;
290+
boolneedquotes;
291+
292+
/*
293+
* If the string is one or more plain ASCII characters, no need to quote
294+
* it. This is quite conservative, but better safe than sorry.
295+
*/
296+
needquotes= true;
297+
for (s=str;*s;s++)
298+
{
299+
if (!((*s >='a'&&*s <='z')|| (*s >='A'&&*s <='Z')||
300+
(*s >='0'&&*s <='9')||*s=='_'||*s=='.'))
301+
{
302+
needquotes= true;
303+
break;
304+
}
305+
needquotes= false;
306+
}
307+
308+
if (needquotes)
309+
{
310+
appendPQExpBufferChar(buf,'\'');
311+
while (*str)
312+
{
313+
/* ' and \ must be escaped by to \' and \\ */
314+
if (*str=='\''||*str=='\\')
315+
appendPQExpBufferChar(buf,'\\');
316+
317+
appendPQExpBufferChar(buf,*str);
318+
str++;
319+
}
320+
appendPQExpBufferChar(buf,'\'');
321+
}
322+
else
323+
appendPQExpBufferStr(buf,str);
324+
}
325+
326+
327+
/*
328+
* Append a psql meta-command that connects to the given database with the
329+
* then-current connection's user, host and port.
330+
*/
331+
void
332+
appendPsqlMetaConnect(PQExpBufferbuf,constchar*dbname)
333+
{
334+
constchar*s;
335+
boolcomplex;
336+
337+
/*
338+
* If the name is plain ASCII characters, emit a trivial "\connect "foo"".
339+
* For other names, even many not technically requiring it, skip to the
340+
* general case. No database has a zero-length name.
341+
*/
342+
complex= false;
343+
for (s=dbname;*s;s++)
344+
{
345+
if (*s=='\n'||*s=='\r')
346+
{
347+
fprintf(stderr,
348+
_("database name contains a newline or carriage return: \"%s\"\n"),
349+
dbname);
350+
exit(EXIT_FAILURE);
351+
}
352+
353+
if (!((*s >='a'&&*s <='z')|| (*s >='A'&&*s <='Z')||
354+
(*s >='0'&&*s <='9')||*s=='_'||*s=='.'))
355+
{
356+
complex= true;
357+
}
358+
}
359+
360+
appendPQExpBufferStr(buf,"\\connect ");
361+
if (complex)
362+
{
363+
PQExpBufferDataconnstr;
364+
365+
initPQExpBuffer(&connstr);
366+
appendPQExpBuffer(&connstr,"dbname=");
367+
appendConnStrVal(&connstr,dbname);
368+
369+
appendPQExpBuffer(buf,"-reuse-previous=on ");
370+
371+
/*
372+
* As long as the name does not contain a newline, SQL identifier
373+
* quoting satisfies the psql meta-command parser. Prefer not to
374+
* involve psql-interpreted single quotes, which behaved differently
375+
* before PostgreSQL 9.2.
376+
*/
377+
appendPQExpBufferStr(buf,quote_identifier(connstr.data));
378+
379+
termPQExpBuffer(&connstr);
380+
}
381+
else
382+
appendPQExpBufferStr(buf,quote_identifier(dbname));
383+
appendPQExpBufferChar(buf,'\n');
384+
}
385+
386+
183387
/*
184388
* get_user_info()
185389
* (copied from initdb.c) find the current user

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp