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

Commit286c8bc

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 parent8adff37 commit286c8bc

File tree

21 files changed

+656
-207
lines changed

21 files changed

+656
-207
lines changed

‎src/bin/pg_basebackup/streamutil.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,15 @@ GetConnection(void)
6464
PQconninfoOption*conn_opt;
6565
char*err_msg=NULL;
6666

67+
/* pg_recvlogical uses dbname only; others use connection_string only. */
68+
Assert(dbname==NULL||connection_string==NULL);
69+
6770
/*
6871
* Merge the connection info inputs given in form of connection string,
6972
* options and default values (dbname=replication, replication=true, etc.)
73+
* Explicitly discard any dbname value in the connection string;
74+
* otherwise, PQconnectdbParams() would interpret that value as being
75+
* itself a connection string.
7076
*/
7177
i=0;
7278
if (connection_string)
@@ -80,7 +86,8 @@ GetConnection(void)
8086

8187
for (conn_opt=conn_opts;conn_opt->keyword!=NULL;conn_opt++)
8288
{
83-
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0')
89+
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0'&&
90+
strcmp(conn_opt->keyword,"dbname")!=0)
8491
argcount++;
8592
}
8693

@@ -89,7 +96,8 @@ GetConnection(void)
8996

9097
for (conn_opt=conn_opts;conn_opt->keyword!=NULL;conn_opt++)
9198
{
92-
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0')
99+
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0'&&
100+
strcmp(conn_opt->keyword,"dbname")!=0)
93101
{
94102
keywords[i]=conn_opt->keyword;
95103
values[i]=conn_opt->val;

‎src/bin/pg_dump/dumputils.c

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,210 @@ appendStringLiteralDQ(PQExpBuffer buf, const char *str, const char *dqprefix)
339339
}
340340

341341

342+
/*
343+
* Append the given string to the shell command being built in the buffer,
344+
* with suitable shell-style quoting to create exactly one argument.
345+
*
346+
* Forbid LF or CR characters, which have scant practical use beyond designing
347+
* security breaches. The Windows command shell is unusable as a conduit for
348+
* arguments containing LF or CR characters. A future major release should
349+
* reject those characters in CREATE ROLE and CREATE DATABASE, because use
350+
* there eventually leads to errors here.
351+
*/
352+
void
353+
appendShellString(PQExpBufferbuf,constchar*str)
354+
{
355+
constchar*p;
356+
357+
#ifndefWIN32
358+
appendPQExpBufferChar(buf,'\'');
359+
for (p=str;*p;p++)
360+
{
361+
if (*p=='\n'||*p=='\r')
362+
{
363+
fprintf(stderr,
364+
_("shell command argument contains a newline or carriage return: \"%s\"\n"),
365+
str);
366+
exit(EXIT_FAILURE);
367+
}
368+
369+
if (*p=='\'')
370+
appendPQExpBufferStr(buf,"'\"'\"'");
371+
else
372+
appendPQExpBufferChar(buf,*p);
373+
}
374+
appendPQExpBufferChar(buf,'\'');
375+
#else/* WIN32 */
376+
intbackslash_run_length=0;
377+
378+
/*
379+
* A Windows system() argument experiences two layers of interpretation.
380+
* First, cmd.exe interprets the string. Its behavior is undocumented,
381+
* but a caret escapes any byte except LF or CR that would otherwise have
382+
* special meaning. Handling of a caret before LF or CR differs between
383+
* "cmd.exe /c" and other modes, and it is unusable here.
384+
*
385+
* Second, the new process parses its command line to construct argv (see
386+
* https://msdn.microsoft.com/en-us/library/17w5ykft.aspx). This treats
387+
* backslash-double quote sequences specially.
388+
*/
389+
appendPQExpBufferStr(buf,"^\"");
390+
for (p=str;*p;p++)
391+
{
392+
if (*p=='\n'||*p=='\r')
393+
{
394+
fprintf(stderr,
395+
_("shell command argument contains a newline or carriage return: \"%s\"\n"),
396+
str);
397+
exit(EXIT_FAILURE);
398+
}
399+
400+
/* Change N backslashes before a double quote to 2N+1 backslashes. */
401+
if (*p=='"')
402+
{
403+
while (backslash_run_length)
404+
{
405+
appendPQExpBufferStr(buf,"^\\");
406+
backslash_run_length--;
407+
}
408+
appendPQExpBufferStr(buf,"^\\");
409+
}
410+
elseif (*p=='\\')
411+
backslash_run_length++;
412+
else
413+
backslash_run_length=0;
414+
415+
/*
416+
* Decline to caret-escape the most mundane characters, to ease
417+
* debugging and lest we approach the command length limit.
418+
*/
419+
if (!((*p >='a'&&*p <='z')||
420+
(*p >='A'&&*p <='Z')||
421+
(*p >='0'&&*p <='9')))
422+
appendPQExpBufferChar(buf,'^');
423+
appendPQExpBufferChar(buf,*p);
424+
}
425+
426+
/*
427+
* Change N backslashes at end of argument to 2N backslashes, because they
428+
* precede the double quote that terminates the argument.
429+
*/
430+
while (backslash_run_length)
431+
{
432+
appendPQExpBufferStr(buf,"^\\");
433+
backslash_run_length--;
434+
}
435+
appendPQExpBufferStr(buf,"^\"");
436+
#endif/* WIN32 */
437+
}
438+
439+
440+
/*
441+
* Append the given string to the buffer, with suitable quoting for passing
442+
* the string as a value, in a keyword/pair value in a libpq connection
443+
* string
444+
*/
445+
void
446+
appendConnStrVal(PQExpBufferbuf,constchar*str)
447+
{
448+
constchar*s;
449+
boolneedquotes;
450+
451+
/*
452+
* If the string is one or more plain ASCII characters, no need to quote
453+
* it. This is quite conservative, but better safe than sorry.
454+
*/
455+
needquotes= true;
456+
for (s=str;*s;s++)
457+
{
458+
if (!((*s >='a'&&*s <='z')|| (*s >='A'&&*s <='Z')||
459+
(*s >='0'&&*s <='9')||*s=='_'||*s=='.'))
460+
{
461+
needquotes= true;
462+
break;
463+
}
464+
needquotes= false;
465+
}
466+
467+
if (needquotes)
468+
{
469+
appendPQExpBufferChar(buf,'\'');
470+
while (*str)
471+
{
472+
/* ' and \ must be escaped by to \' and \\ */
473+
if (*str=='\''||*str=='\\')
474+
appendPQExpBufferChar(buf,'\\');
475+
476+
appendPQExpBufferChar(buf,*str);
477+
str++;
478+
}
479+
appendPQExpBufferChar(buf,'\'');
480+
}
481+
else
482+
appendPQExpBufferStr(buf,str);
483+
}
484+
485+
486+
/*
487+
* Append a psql meta-command that connects to the given database with the
488+
* then-current connection's user, host and port.
489+
*/
490+
void
491+
appendPsqlMetaConnect(PQExpBufferbuf,constchar*dbname)
492+
{
493+
constchar*s;
494+
boolcomplex;
495+
496+
/*
497+
* If the name is plain ASCII characters, emit a trivial "\connect "foo"".
498+
* For other names, even many not technically requiring it, skip to the
499+
* general case. No database has a zero-length name.
500+
*/
501+
complex= false;
502+
for (s=dbname;*s;s++)
503+
{
504+
if (*s=='\n'||*s=='\r')
505+
{
506+
fprintf(stderr,
507+
_("database name contains a newline or carriage return: \"%s\"\n"),
508+
dbname);
509+
exit(EXIT_FAILURE);
510+
}
511+
512+
if (!((*s >='a'&&*s <='z')|| (*s >='A'&&*s <='Z')||
513+
(*s >='0'&&*s <='9')||*s=='_'||*s=='.'))
514+
{
515+
complex= true;
516+
}
517+
}
518+
519+
appendPQExpBufferStr(buf,"\\connect ");
520+
if (complex)
521+
{
522+
PQExpBufferDataconnstr;
523+
524+
initPQExpBuffer(&connstr);
525+
appendPQExpBuffer(&connstr,"dbname=");
526+
appendConnStrVal(&connstr,dbname);
527+
528+
appendPQExpBuffer(buf,"-reuse-previous=on ");
529+
530+
/*
531+
* As long as the name does not contain a newline, SQL identifier
532+
* quoting satisfies the psql meta-command parser. Prefer not to
533+
* involve psql-interpreted single quotes, which behaved differently
534+
* before PostgreSQL 9.2.
535+
*/
536+
appendPQExpBufferStr(buf,fmtId(connstr.data));
537+
538+
termPQExpBuffer(&connstr);
539+
}
540+
else
541+
appendPQExpBufferStr(buf,fmtId(dbname));
542+
appendPQExpBufferChar(buf,'\n');
543+
}
544+
545+
342546
/*
343547
* Convert a bytea value (presented as raw bytes) to an SQL string literal
344548
* and append it to the given buffer. We assume the specified

‎src/bin/pg_dump/dumputils.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,9 @@ extern void appendStringLiteralDQ(PQExpBuffer buf, const char *str,
8181
externvoidappendByteaLiteral(PQExpBufferbuf,
8282
constunsignedchar*str,size_tlength,
8383
boolstd_strings);
84+
externvoidappendShellString(PQExpBufferbuf,constchar*str);
85+
externvoidappendConnStrVal(PQExpBufferbuf,constchar*str);
86+
externvoidappendPsqlMetaConnect(PQExpBufferbuf,constchar*dbname);
8487
externboolparsePGArray(constchar*atext,char***itemarray,int*nitems);
8588
externboolbuildACLCommands(constchar*name,constchar*subname,
8689
constchar*type,constchar*acls,constchar*owner,

‎src/bin/pg_dump/pg_backup.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ typedef struct _restoreOptions
102102
SimpleStringListtableNames;
103103

104104
intuseDB;
105-
char*dbname;
105+
char*dbname;/* subject to expand_dbname */
106106
char*pgport;
107107
char*pghost;
108108
char*username;
@@ -120,7 +120,7 @@ typedef struct _restoreOptions
120120

121121
typedefstruct_dumpOptions
122122
{
123-
constchar*dbname;
123+
constchar*dbname;/* subject to expand_dbname */
124124
constchar*pghost;
125125
constchar*pgport;
126126
constchar*username;

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -762,9 +762,16 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
762762
/* If we created a DB, connect to it... */
763763
if (strcmp(te->desc,"DATABASE")==0)
764764
{
765+
PQExpBufferDataconnstr;
766+
767+
initPQExpBuffer(&connstr);
768+
appendPQExpBufferStr(&connstr,"dbname=");
769+
appendConnStrVal(&connstr,te->tag);
770+
/* Abandon struct, but keep its buffer until process exit. */
771+
765772
ahlog(AH,1,"connecting to new database \"%s\"\n",te->tag);
766773
_reconnectToDB(AH,te->tag);
767-
ropt->dbname=pg_strdup(te->tag);
774+
ropt->dbname=connstr.data;
768775
}
769776
}
770777

@@ -2921,12 +2928,17 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname)
29212928
ReconnectToServer(AH,dbname,NULL);
29222929
else
29232930
{
2924-
PQExpBufferqry=createPQExpBuffer();
2931+
if (dbname)
2932+
{
2933+
PQExpBufferDataconnectbuf;
29252934

2926-
appendPQExpBuffer(qry,"\\connect %s\n\n",
2927-
dbname ?fmtId(dbname) :"-");
2928-
ahprintf(AH,"%s",qry->data);
2929-
destroyPQExpBuffer(qry);
2935+
initPQExpBuffer(&connectbuf);
2936+
appendPsqlMetaConnect(&connectbuf,dbname);
2937+
ahprintf(AH,"%s\n",connectbuf.data);
2938+
termPQExpBuffer(&connectbuf);
2939+
}
2940+
else
2941+
ahprintf(AH,"%s\n","\\connect -\n");
29302942
}
29312943

29322944
/*
@@ -4400,7 +4412,7 @@ CloneArchive(ArchiveHandle *AH)
44004412
}
44014413
else
44024414
{
4403-
char*dbname;
4415+
PQExpBufferDataconnstr;
44044416
char*pghost;
44054417
char*pgport;
44064418
char*username;
@@ -4413,14 +4425,18 @@ CloneArchive(ArchiveHandle *AH)
44134425
* because all just return a pointer and do not actually send/receive
44144426
* any data to/from the database.
44154427
*/
4416-
dbname=PQdb(AH->connection);
4428+
initPQExpBuffer(&connstr);
4429+
appendPQExpBuffer(&connstr,"dbname=");
4430+
appendConnStrVal(&connstr,PQdb(AH->connection));
44174431
pghost=PQhost(AH->connection);
44184432
pgport=PQport(AH->connection);
44194433
username=PQuser(AH->connection);
44204434

44214435
/* this also sets clone->connection */
4422-
ConnectDatabase((Archive*)clone,dbname,pghost,pgport,username,TRI_NO);
4436+
ConnectDatabase((Archive*)clone,connstr.data,
4437+
pghost,pgport,username,TRI_NO);
44234438

4439+
termPQExpBuffer(&connstr);
44244440
/* setupDumpWorker will fix up connection state */
44254441
}
44264442

‎src/bin/pg_dump/pg_backup_db.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
128128
staticPGconn*
129129
_connectDB(ArchiveHandle*AH,constchar*reqdb,constchar*requser)
130130
{
131+
PQExpBufferDataconnstr;
131132
PGconn*newConn;
132133
constchar*newdb;
133134
constchar*newuser;
@@ -156,6 +157,10 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
156157
exit_horribly(modulename,"out of memory\n");
157158
}
158159

160+
initPQExpBuffer(&connstr);
161+
appendPQExpBuffer(&connstr,"dbname=");
162+
appendConnStrVal(&connstr,newdb);
163+
159164
do
160165
{
161166
constchar*keywords[7];
@@ -170,7 +175,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
170175
keywords[3]="password";
171176
values[3]=password;
172177
keywords[4]="dbname";
173-
values[4]=newdb;
178+
values[4]=connstr.data;
174179
keywords[5]="fallback_application_name";
175180
values[5]=progname;
176181
keywords[6]=NULL;
@@ -222,6 +227,8 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
222227
if (password)
223228
free(password);
224229

230+
termPQExpBuffer(&connstr);
231+
225232
/* check for version mismatch */
226233
_check_database_version(AH);
227234

0 commit comments

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