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

Commitfcd15f1

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 parent41f18f0 commitfcd15f1

File tree

20 files changed

+315
-67
lines changed

20 files changed

+315
-67
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/pg_backup.h

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

105105
intuseDB;
106-
char*dbname;
106+
char*dbname;/* subject to expand_dbname */
107107
char*pgport;
108108
char*pghost;
109109
char*username;
@@ -121,7 +121,7 @@ typedef struct _restoreOptions
121121

122122
typedefstruct_dumpOptions
123123
{
124-
constchar*dbname;
124+
constchar*dbname;/* subject to expand_dbname */
125125
constchar*pghost;
126126
constchar*pgport;
127127
constchar*username;

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -771,9 +771,16 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
771771
/* If we created a DB, connect to it... */
772772
if (strcmp(te->desc,"DATABASE")==0)
773773
{
774+
PQExpBufferDataconnstr;
775+
776+
initPQExpBuffer(&connstr);
777+
appendPQExpBufferStr(&connstr,"dbname=");
778+
appendConnStrVal(&connstr,te->tag);
779+
/* Abandon struct, but keep its buffer until process exit. */
780+
774781
ahlog(AH,1,"connecting to new database \"%s\"\n",te->tag);
775782
_reconnectToDB(AH,te->tag);
776-
ropt->dbname=pg_strdup(te->tag);
783+
ropt->dbname=connstr.data;
777784
}
778785
}
779786

@@ -2984,12 +2991,17 @@ _reconnectToDB(ArchiveHandle *AH, const char *dbname)
29842991
ReconnectToServer(AH,dbname,NULL);
29852992
else
29862993
{
2987-
PQExpBufferqry=createPQExpBuffer();
2994+
if (dbname)
2995+
{
2996+
PQExpBufferDataconnectbuf;
29882997

2989-
appendPQExpBuffer(qry,"\\connect %s\n\n",
2990-
dbname ?fmtId(dbname) :"-");
2991-
ahprintf(AH,"%s",qry->data);
2992-
destroyPQExpBuffer(qry);
2998+
initPQExpBuffer(&connectbuf);
2999+
appendPsqlMetaConnect(&connectbuf,dbname);
3000+
ahprintf(AH,"%s\n",connectbuf.data);
3001+
termPQExpBuffer(&connectbuf);
3002+
}
3003+
else
3004+
ahprintf(AH,"%s\n","\\connect -\n");
29933005
}
29943006

29953007
/*
@@ -4463,7 +4475,7 @@ CloneArchive(ArchiveHandle *AH)
44634475
}
44644476
else
44654477
{
4466-
char*dbname;
4478+
PQExpBufferDataconnstr;
44674479
char*pghost;
44684480
char*pgport;
44694481
char*username;
@@ -4476,14 +4488,18 @@ CloneArchive(ArchiveHandle *AH)
44764488
* because all just return a pointer and do not actually send/receive
44774489
* any data to/from the database.
44784490
*/
4479-
dbname=PQdb(AH->connection);
4491+
initPQExpBuffer(&connstr);
4492+
appendPQExpBuffer(&connstr,"dbname=");
4493+
appendConnStrVal(&connstr,PQdb(AH->connection));
44804494
pghost=PQhost(AH->connection);
44814495
pgport=PQport(AH->connection);
44824496
username=PQuser(AH->connection);
44834497

44844498
/* this also sets clone->connection */
4485-
ConnectDatabase((Archive*)clone,dbname,pghost,pgport,username,TRI_NO);
4499+
ConnectDatabase((Archive*)clone,connstr.data,
4500+
pghost,pgport,username,TRI_NO);
44864501

4502+
termPQExpBuffer(&connstr);
44874503
/* setupDumpWorker will fix up connection state */
44884504
}
44894505

‎src/bin/pg_dump/pg_backup_db.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include"postgres_fe.h"
1313

1414
#include"dumputils.h"
15+
#include"fe_utils/string_utils.h"
1516
#include"parallel.h"
1617
#include"pg_backup_archiver.h"
1718
#include"pg_backup_db.h"
@@ -128,6 +129,7 @@ ReconnectToServer(ArchiveHandle *AH, const char *dbname, const char *username)
128129
staticPGconn*
129130
_connectDB(ArchiveHandle*AH,constchar*reqdb,constchar*requser)
130131
{
132+
PQExpBufferDataconnstr;
131133
PGconn*newConn;
132134
constchar*newdb;
133135
constchar*newuser;
@@ -156,6 +158,10 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
156158
exit_horribly(modulename,"out of memory\n");
157159
}
158160

161+
initPQExpBuffer(&connstr);
162+
appendPQExpBuffer(&connstr,"dbname=");
163+
appendConnStrVal(&connstr,newdb);
164+
159165
do
160166
{
161167
constchar*keywords[7];
@@ -170,7 +176,7 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
170176
keywords[3]="password";
171177
values[3]=password;
172178
keywords[4]="dbname";
173-
values[4]=newdb;
179+
values[4]=connstr.data;
174180
keywords[5]="fallback_application_name";
175181
values[5]=progname;
176182
keywords[6]=NULL;
@@ -222,6 +228,8 @@ _connectDB(ArchiveHandle *AH, const char *reqdb, const char *requser)
222228
if (password)
223229
free(password);
224230

231+
termPQExpBuffer(&connstr);
232+
225233
/* check for version mismatch */
226234
_check_database_version(AH);
227235

‎src/bin/pg_dump/pg_dumpall.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1507,7 +1507,7 @@ dumpCreateDB(PGconn *conn)
15071507
fdbname,fmtId(dbtablespace));
15081508

15091509
/* connect to original database */
1510-
appendPQExpBuffer(buf,"\\connect %s\n",fdbname);
1510+
appendPsqlMetaConnect(buf,dbname);
15111511
}
15121512

15131513
if (binary_upgrade)
@@ -1740,11 +1740,15 @@ dumpDatabases(PGconn *conn)
17401740
intret;
17411741

17421742
char*dbname=PQgetvalue(res,i,0);
1743+
PQExpBufferDataconnectbuf;
17431744

17441745
if (verbose)
17451746
fprintf(stderr,_("%s: dumping database \"%s\"...\n"),progname,dbname);
17461747

1747-
fprintf(OPF,"\\connect %s\n\n",fmtId(dbname));
1748+
initPQExpBuffer(&connectbuf);
1749+
appendPsqlMetaConnect(&connectbuf,dbname);
1750+
fprintf(OPF,"%s\n",connectbuf.data);
1751+
termPQExpBuffer(&connectbuf);
17481752

17491753
/*
17501754
* Restore will need to write to the target cluster. This connection
@@ -1900,7 +1904,9 @@ connectDatabase(const char *dbname, const char *connection_string,
19001904

19011905
/*
19021906
* Merge the connection info inputs given in form of connection string
1903-
* and other options.
1907+
* and other options. Explicitly discard any dbname value in the
1908+
* connection string; otherwise, PQconnectdbParams() would interpret
1909+
* that value as being itself a connection string.
19041910
*/
19051911
if (connection_string)
19061912
{
@@ -1913,7 +1919,8 @@ connectDatabase(const char *dbname, const char *connection_string,
19131919

19141920
for (conn_opt=conn_opts;conn_opt->keyword!=NULL;conn_opt++)
19151921
{
1916-
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0')
1922+
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0'&&
1923+
strcmp(conn_opt->keyword,"dbname")!=0)
19171924
argcount++;
19181925
}
19191926

@@ -1922,7 +1929,8 @@ connectDatabase(const char *dbname, const char *connection_string,
19221929

19231930
for (conn_opt=conn_opts;conn_opt->keyword!=NULL;conn_opt++)
19241931
{
1925-
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0')
1932+
if (conn_opt->val!=NULL&&conn_opt->val[0]!='\0'&&
1933+
strcmp(conn_opt->keyword,"dbname")!=0)
19261934
{
19271935
keywords[i]=conn_opt->keyword;
19281936
values[i]=conn_opt->val;

‎src/bin/pg_upgrade/Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,12 @@ OBJS = check.o controldata.o dump.o exec.o file.o function.o info.o \
1212
tablespace.o util.o version.o$(WIN32RES)
1313

1414
overrideCPPFLAGS := -DDLSUFFIX=\"$(DLSUFFIX)\" -I$(srcdir) -I$(libpq_srcdir)$(CPPFLAGS)
15+
LDFLAGS += -L$(top_builddir)/src/fe_utils -lpgfeutils -lpq
1516

1617

1718
all: pg_upgrade
1819

19-
pg_upgrade:$(OBJS) | submake-libpq submake-libpgport
20+
pg_upgrade:$(OBJS) | submake-libpq submake-libpgport submake-libpgfeutils
2021
$(CC)$(CFLAGS)$^$(libpq_pgport)$(LDFLAGS)$(LDFLAGS_EX)$(LIBS) -o$@$(X)
2122

2223
install: all installdirs

‎src/bin/pg_upgrade/check.c

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include"postgres_fe.h"
1111

1212
#include"catalog/pg_authid.h"
13+
#include"fe_utils/string_utils.h"
1314
#include"mb/pg_wchar.h"
1415
#include"pg_upgrade.h"
1516

@@ -414,12 +415,17 @@ void
414415
create_script_for_cluster_analyze(char**analyze_script_file_name)
415416
{
416417
FILE*script=NULL;
417-
char*user_specification="";
418+
PQExpBufferDatauser_specification;
418419

419420
prep_status("Creating script to analyze new cluster");
420421

422+
initPQExpBuffer(&user_specification);
421423
if (os_info.user_specified)
422-
user_specification=psprintf("-U \"%s\" ",os_info.user);
424+
{
425+
appendPQExpBufferStr(&user_specification,"-U ");
426+
appendShellString(&user_specification,os_info.user);
427+
appendPQExpBufferChar(&user_specification,' ');
428+
}
423429

424430
*analyze_script_file_name=psprintf("%sanalyze_new_cluster.%s",
425431
SCRIPT_PREFIX,SCRIPT_EXT);
@@ -459,18 +465,18 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
459465
fprintf(script,"echo %sthis script and run:%s\n",
460466
ECHO_QUOTE,ECHO_QUOTE);
461467
fprintf(script,"echo %s \"%s/vacuumdb\" %s--all %s%s\n",ECHO_QUOTE,
462-
new_cluster.bindir,user_specification,
468+
new_cluster.bindir,user_specification.data,
463469
/* Did we copy the free space files? */
464470
(GET_MAJOR_VERSION(old_cluster.major_version) >=804) ?
465471
"--analyze-only" :"--analyze",ECHO_QUOTE);
466472
fprintf(script,"echo%s\n\n",ECHO_BLANK);
467473

468474
fprintf(script,"\"%s/vacuumdb\" %s--all --analyze-in-stages\n",
469-
new_cluster.bindir,user_specification);
475+
new_cluster.bindir,user_specification.data);
470476
/* Did we copy the free space files? */
471477
if (GET_MAJOR_VERSION(old_cluster.major_version)<804)
472478
fprintf(script,"\"%s/vacuumdb\" %s--all\n",new_cluster.bindir,
473-
user_specification);
479+
user_specification.data);
474480

475481
fprintf(script,"echo%s\n\n",ECHO_BLANK);
476482
fprintf(script,"echo %sDone%s\n",
@@ -484,8 +490,7 @@ create_script_for_cluster_analyze(char **analyze_script_file_name)
484490
*analyze_script_file_name,getErrorText());
485491
#endif
486492

487-
if (os_info.user_specified)
488-
pg_free(user_specification);
493+
termPQExpBuffer(&user_specification);
489494

490495
check_ok();
491496
}

‎src/bin/pg_upgrade/dump.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
#include"pg_upgrade.h"
1313

1414
#include<sys/types.h>
15+
#include"fe_utils/string_utils.h"
1516

1617

1718
void
@@ -46,17 +47,28 @@ generate_old_dump(void)
4647
charsql_file_name[MAXPGPATH],
4748
log_file_name[MAXPGPATH];
4849
DbInfo*old_db=&old_cluster.dbarr.dbs[dbnum];
50+
PQExpBufferDataconnstr,
51+
escaped_connstr;
52+
53+
initPQExpBuffer(&connstr);
54+
appendPQExpBuffer(&connstr,"dbname=");
55+
appendConnStrVal(&connstr,old_db->db_name);
56+
initPQExpBuffer(&escaped_connstr);
57+
appendShellString(&escaped_connstr,connstr.data);
58+
termPQExpBuffer(&connstr);
4959

5060
pg_log(PG_STATUS,"%s",old_db->db_name);
5161
snprintf(sql_file_name,sizeof(sql_file_name),DB_DUMP_FILE_MASK,old_db->db_oid);
5262
snprintf(log_file_name,sizeof(log_file_name),DB_DUMP_LOG_FILE_MASK,old_db->db_oid);
5363

5464
parallel_exec_prog(log_file_name,NULL,
5565
"\"%s/pg_dump\" %s --schema-only --quote-all-identifiers "
56-
"--binary-upgrade --format=custom %s --file=\"%s\"\"%s\"",
66+
"--binary-upgrade --format=custom %s --file=\"%s\"%s",
5767
new_cluster.bindir,cluster_conn_opts(&old_cluster),
5868
log_opts.verbose ?"--verbose" :"",
59-
sql_file_name,old_db->db_name);
69+
sql_file_name,escaped_connstr.data);
70+
71+
termPQExpBuffer(&escaped_connstr);
6072
}
6173

6274
/* reap all children */

‎src/bin/pg_upgrade/pg_upgrade.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838

3939
#include"pg_upgrade.h"
4040
#include"common/restricted_token.h"
41+
#include"fe_utils/string_utils.h"
4142

4243
#ifdefHAVE_LANGINFO_H
4344
#include<langinfo.h>
@@ -305,6 +306,15 @@ create_new_objects(void)
305306
charsql_file_name[MAXPGPATH],
306307
log_file_name[MAXPGPATH];
307308
DbInfo*old_db=&old_cluster.dbarr.dbs[dbnum];
309+
PQExpBufferDataconnstr,
310+
escaped_connstr;
311+
312+
initPQExpBuffer(&connstr);
313+
appendPQExpBuffer(&connstr,"dbname=");
314+
appendConnStrVal(&connstr,old_db->db_name);
315+
initPQExpBuffer(&escaped_connstr);
316+
appendShellString(&escaped_connstr,connstr.data);
317+
termPQExpBuffer(&connstr);
308318

309319
pg_log(PG_STATUS,"%s",old_db->db_name);
310320
snprintf(sql_file_name,sizeof(sql_file_name),DB_DUMP_FILE_MASK,old_db->db_oid);
@@ -316,11 +326,13 @@ create_new_objects(void)
316326
*/
317327
parallel_exec_prog(log_file_name,
318328
NULL,
319-
"\"%s/pg_restore\" %s --exit-on-error --verbose --dbname\"%s\" \"%s\"",
329+
"\"%s/pg_restore\" %s --exit-on-error --verbose --dbname%s \"%s\"",
320330
new_cluster.bindir,
321331
cluster_conn_opts(&new_cluster),
322-
old_db->db_name,
332+
escaped_connstr.data,
323333
sql_file_name);
334+
335+
termPQExpBuffer(&escaped_connstr);
324336
}
325337

326338
/* reap all children */

0 commit comments

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