8000 Fix insecure parsing of server command-line switches. · danielcode/postgres@a6e0cd7 · GitHub
[go: up one dir, main page]
Skip to content

Commit a6e0cd7

Browse files
tglsfdctglsfdc
committed
Fix insecure parsing of server command-line switches.
An oversight in commit e710b65 allowed database names beginning with "-" to be treated as though they were secure command-line switches; and this switch processing occurs before client authentication, so that even an unprivileged remote attacker could exploit the bug, needing only connectivity to the postmaster's port. Assorted exploits for this are possible, some requiring a valid database login, some not. The worst known problem is that the "-r" switch can be invoked to redirect the process's stderr output, so that subsequent error messages will be appended to any file the server can write. This can for example be used to corrupt the server's configuration files, so that it will fail when next restarted. Complete destruction of database tables is also possible. Fix by keeping the database name extracted from a startup packet fully separate from command-line switches, as had already been done with the user name field. The Postgres project thanks Mitsumasa Kondo for discovering this bug, Kyotaro Horiguchi for drafting the fix, and Noah Misch for recognizing the full extent of the danger. Security: CVE-2013-1899
1 parent e5fdb8f commit a6e0cd7

File tree

5 files changed

+29
-32
lines changed

5 files changed

+29
-32
lines changed

‎src/backend/main/main.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ main(int argc, char *argv[])
194194
exit(GucInfoMain());
195195

196196
if (argc > 1 && strcmp(argv[1], "--single") == 0)
197-
exit(PostgresMain(argc, argv, get_current_username(progname)));
197+
exit(PostgresMain(argc, argv, NULL, get_current_username(progname)));
198198

199199
exit(PostmasterMain(argc, argv));
200200
}

‎src/backend/postmaster/postmaster.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3573,7 +3573,7 @@ BackendRun(Port *port)
35733573
* from ExtraOptions is (strlen(ExtraOptions) + 1) / 2; see
35743574
* pg_split_opts().
35753575
*/
3576-
maxac = 5; /* for fixed args supplied below */
3576+
maxac = 2; /* for fixed args supplied below */
35773577
maxac += (strlen(ExtraOptions) + 1) / 2;
35783578

35793579
av = (char **) MemoryContextAlloc(TopMemoryContext,
@@ -3589,11 +3589,6 @@ BackendRun(Port *port)
35893589
*/
35903590
pg_split_opts(av, &ac, ExtraOptions);
35913591

3592-
/*
3593-
* Tell the backend which database to use.
3594-
*/
3595-
av[ac++] = port->database_name;
3596-
35973592
av[ac] = NULL;
35983593

35993594
Assert(ac < maxac);
@@ -3616,7 +3611,7 @@ BackendRun(Port *port)
36163611
*/
36173612
MemoryContextSwitchTo(TopMemoryContext);
36183613

3619-
return (PostgresMain(ac, av, port->user_name));
3614+
return PostgresMain(ac, av, port->database_name, port->user_name);
36203615
}
36213616

36223617

‎src/backend/tcop/postgres.c

Lines changed: 21 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3230,13 +3230,14 @@ get_stats_option_name(const char *arg)
32303230
* coming from the client, or PGC_SUSET for insecure options coming from
32313231
* a superuser client.
32323232
*
3233-
* Returns the database name extracted from the command line, if any.
3233+
* If a database name is present in the command line arguments, it's
3234+
* returned into *dbname (this is allowed only if *dbname is initially NULL).
32343235
* ----------------------------------------------------------------
32353236
*/
3236-
const char *
3237-
process_postgres_switches(int argc, char *argv[], GucContext ctx)
3237+
void
3238+
process_postgres_switches(int argc, char *argv[], GucContext ctx,
3239+
const char **dbname)
32383240
{
3239-
const char *dbname;
32403241
bool secure = (ctx == PGC_POSTMASTER);
32413242
int errs = 0;
32423243
GucSource gucsource;
@@ -3287,7 +3288,8 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
32873288

32883289
case 'b':
32893290
/* Undocumented flag used for binary upgrades */
3290-
IsBinaryUpgrade = true;
3291+
if (secure)
3292+
IsBinaryUpgrade = true;
32913293
break;
32923294

32933295
case 'C':
@@ -3304,7 +3306,8 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
33043306
break;
33053307

33063308
case 'E':
3307-
EchoQuery = true;
3309+
if (secure)
3310+
EchoQuery = true;
33083311
break;
33093312

33103313
case 'e':
@@ -3329,7 +3332,8 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
33293332
break;
33303333

33313334
case 'j':
3332-
UseNewLine = 0;
3335+
if (secure)
3336+
UseNewLine = 0;
33333337
break;
33343338

33353339
case 'k':
@@ -3447,13 +3451,10 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
34473451
}
34483452

34493453
/*
3450-
* Should be no more arguments except an optional database name, and
3451-
* that's only in the secure case.
3454+
* Optional database name should be there only if *dbname is NULL.
34523455
*/
3453-
if (!errs && secure && argc - optind >= 1)
3454-
dbname = strdup(argv[optind++]);
3455-
else
3456-
dbname = NULL;
3456+
if (!errs && dbname && *dbname == NULL && argc - optind >= 1)
3457+
*dbname = strdup(argv[optind++]);
34573458

34583459
if (errs || argc != optind)
34593460
{
@@ -3482,8 +3483,6 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
34823483
#ifdef HAVE_INT_OPTRESET
34833484
optreset = 1; /* some systems need this too */
34843485
#endif
3485-
3486-
return dbname;
34873486
}
34883487

34893488

@@ -3493,14 +3492,16 @@ process_postgres_switches(int argc, char *argv[], GucContext ctx)
34933492
*
34943493
* argc/argv are the command line arguments to be used. (When being forked
34953494
* by the postmaster, these are not the original argv array of the process.)
3496-
* username is the (possibly authenticated) PostgreSQL user name to be used
3497-
* for the session.
3495+
* dbname is the name of the database to connect to, or NULL if the database
3496+
* name should be extracted from the command line arguments or defaulted.
3497+
* username is the PostgreSQL user name to be used for the session.
34983498
* ----------------------------------------------------------------
34993499
*/
35003500
int
3501-
PostgresMain(int argc, char *argv[], const char *username)
3501+
PostgresMain(int argc, char *argv[],
3502+
const char *dbname,
3503+
const char *username)
35023504
{
3503-
const char *dbname;
35043505
int firstchar;
35053506
StringInfoData input_message;
35063507
sigjmp_buf local_sigjmp_buf;
@@ -3547,7 +3548,7 @@ PostgresMain(int argc, char *argv[], const char *username)
35473548
/*
35483549
* Parse command-line options.
35493550
*/
3550-
dbname = process_postgres_switches(argc, argv, PGC_POSTMASTER);
3551+
process_postgres_switches(argc, argv, PGC_POSTMASTER, &dbname);
35513552

35523553
/* Must have gotten a database name, or have a default (the username) */
35533554
if (dbname == NULL)

‎src/backend/utils/init/postinit.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ process_startup_options(Port *port, bool am_superuser)
911911

912912
Assert(ac < maxac);
913913

914-
(void) process_postgres_switches(ac, av, gucctx);
914+
(void) process_postgres_switches(ac, av, gucctx, NULL);
915915
}
916916

917917
/*

‎src/include/tcop/tcopprot.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,10 @@ extern void RecoveryConflictInterrupt(ProcSignalReason reason); /* called from S
6868
* handler */
6969
extern void prepare_for_client_read(void);
7070
extern void client_read_ended(void);
71-
extern const char *process_postgres_switches(int argc, char *argv[],
72-
GucContext ctx);
73-
extern int PostgresMain(int argc, char *argv[], const char *username);
71+
extern void process_postgres_switches(int argc, char *argv[],
72+
GucContext ctx, const char **dbname);
73+
extern int PostgresMain(int argc, char *argv[],
74+
const char *dbname, const char *username);
7475
extern long get_stack_depth_rlimit(void);
7576
extern void ResetUsage(void);
7677
extern void ShowUsage(const char *title);

0 commit comments

Comments
 (0)
0