Skip to content

Commit 303696c

Browse files
tglsfdctglsfdc
committed
Install a data-type-based solution for protecting pg_get_expr().
Since the code underlying pg_get_expr() is not secure against malformed input, and can't practically be made so, we need to prevent miscreants from feeding arbitrary data to it. We can do this securely by declaring pg_get_expr() to take a new datatype "pg_node_tree" and declaring the system catalog columns that hold nodeToString output to be of that type. There is no way at SQL level to create a non-null value of type pg_node_tree. Since the backend-internal operations that fill those catalog columns operate below the SQL level, they are oblivious to the datatype relabeling and don't need any changes.
1 parent 8ab6a6b commit 303696c

File tree

19 files changed

+149
-62
lines changed

19 files changed

+149
-62
lines changed

‎doc/src/sgml/catalogs.sgml

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.227 2010/08/25 18:18:41 petere Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/catalogs.sgml,v 2.228 2010/09/03 01:34:54 tgl Exp $ -->
22
<!--
33
Documentation of the system catalogs, directed toward PostgreSQL developers
44
-->
@@ -797,7 +797,7 @@
797797

798798
<row>
799799
<entry><structfield>adbin</structfield></entry>
800-
<entry><type>text</type></entry>
800+
<entry><type>pg_node_tree</type></entry>
801801
<entry></entry>
802802
<entry>The internal representation of the column default value</entry>
803803
</row>
@@ -1917,7 +1917,7 @@
19171917

19181918
<row>
19191919
<entry><structfield>conbin</structfield></entry>
1920-
<entry><type>text</type></entry>
1920+
<entry><type>pg_node_tree</type></entry>
19211921
<entry></entry>
19221922
<entry>If a check constraint, an internal representation of the expression</entry>
19231923
</row>
@@ -2915,7 +2915,7 @@
29152915

29162916
<row>
29172917
<entry><structfield>indexprs</structfield></entry>
2918-
<entry><type>text</type></entry>
2918+
<entry><type>pg_node_tree</type></entry>
29192919
<entry></entry>
29202920
<entry>
29212921
Expression trees (in <function>nodeToString()</function>
@@ -2928,7 +2928,7 @@
29282928

29292929
<row>
29302930
<entry><structfield>indpred</structfield></entry>
2931-
<entry><type>text</type></entry>
2931+
<entry><type>pg_node_tree</type></entry>
29322932
<entry></entry>
29332933
<entry>
29342934
Expression tree (in <function>nodeToString()</function>
@@ -3980,7 +3980,7 @@
39803980

39813981
<row>
39823982
<entry><structfield>proargdefaults</structfield></entry>
3983-
<entry><type>text</type></entry>
3983+
<entry><type>pg_node_tree</type></entry>
39843984
<entry></entry>
39853985
<entry>
39863986
Expression trees (in <function>nodeToString()</function> representation)
@@ -4129,7 +4129,7 @@
41294129

41304130
<row>
41314131
<entry><structfield>ev_qual</structfield></entry>
4132-
<entry><type>text</type></entry>
4132+
<entry><type>pg_node_tree</type></entry>
41334133
<entry></entry>
41344134
<entry>
41354135
Expression tree (in the form of a
@@ -4140,7 +4140,7 @@
41404140

41414141
<row>
41424142
<entry><structfield>ev_action</structfield></entry>
4143-
<entry><type>text</type></entry>
4143+
<entry><type>pg_node_tree</type></entry>
41444144
<entry></entry>
41454145
<entry>
41464146
Query tree (in the form of a
@@ -4839,7 +4839,7 @@
48394839

48404840
<row>
48414841
<entry><structfield>tgqual</structfield></entry>
4842-
<entry><type>text</type></entry>
4842+
<entry><type>pg_node_tree</type></entry>
48434843
<entry></entry>
48444844
<entry>Expression tree (in <function>nodeToString()</function>
48454845
representation) for the trigger's <literal>WHEN</> condition, or null
@@ -5622,10 +5622,11 @@
56225622

56235623
<row>
56245624
<entry><structfield>typdefaultbin</structfield></entry>
5625-
<entry><type>text</type></entry>
5625+
<entry><type>pg_node_tree</type></entry>
56265626
<entry></entry>
56275627
<entry><para>
5628-
If <structfield>typdefaultbin</> is not null, it is the <function>nodeToString()</function>
5628+
If <structfield>typdefaultbin</> is not null, it is the
5629+
<function>nodeToString()</function>
56295630
representation of a default expression for the type. This is
56305631
only used for domains.
56315632
</para></entry>
@@ -5642,7 +5643,7 @@
56425643
default expression represented by <structfield>typdefaultbin</>. If
56435644
<structfield>typdefaultbin</> is null and <structfield>typdefault</> is
56445645
not, then <structfield>typdefault</> is the external representation of
5645-
the type's default value, which might be fed to the type's input
5646+
the type's default value, which can be fed to the type's input
56465647
converter to produce a constant.
56475648
</para></entry>
56485649
</row>

‎doc/src/sgml/func.sgml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.531 2010/09/01 18:22:29 tgl Exp $ -->
1+
<!-- $PostgreSQL: pgsql/doc/src/sgml/func.sgml,v 1.532 2010/09/03 01:34:54 tgl Exp $ -->
22

33
<chapter id="functions">
44
<title>Functions and Operators</title>
@@ -12746,13 +12746,13 @@ SELECT pg_type_is_visible('myschema.widget'::regtype);
1274612746
<entry>get definition of a constraint</entry>
1274712747
</row>
1274812748
<row>
12749-
<entry><literal><function>pg_get_expr</function>(<parameter>expr_text</parameter>, <parameter>relation_oid</>)</literal></entry>
12749+
<entry><literal><function>pg_get_expr</function>(<parameter>pg_node_tree</parameter>, <parameter>relation_oid</>)</literal></entry>
1275012750
<entry><type>text</type></entry>
1275112751
<entry>decompile internal form of an expression, assuming that any Vars
1275212752
in it refer to the relation indicated by the second parameter</entry>
1275312753
</row>
1275412754
<row>
12755-
<entry><literal><function>pg_get_expr</function>(<parameter>expr_text</parameter>, <parameter>relation_oid</>, <parameter>pretty_bool</>)</literal></entry>
12755+
<entry><literal><function>pg_get_expr</function>(<parameter>pg_node_tree</parameter>, <parameter>relation_oid</>, <parameter>pretty_bool</>)</literal></entry>
1275612756
<entry><type>text</type></entry>
1275712757
<entry>decompile internal form of an expression, assuming that any Vars
1275812758
in it refer to the relation indicated by the second parameter</entry>

‎src/backend/bootstrap/bootstrap.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/bootstrap/bootstrap.c,v 1.261 2010/04/20 01:38:52 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/bootstrap/bootstrap.c,v 1.262 2010/09/03 01:34:55 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -72,7 +72,8 @@ int numattr; /* number of attributes for cur. rel */
7272

7373
/*
7474
* Basic information associated with each type. This is used before
75-
* pg_type is created.
75+
* pg_type is filled, so it has to cover the datatypes used as column types
76+
* in the core "bootstrapped" catalogs.
7677
*
7778
* XXX several of these input/output functions do catalog scans
7879
* (e.g., F_REGPROCIN scans pg_proc). this obviously creates some
@@ -122,6 +123,8 @@ static const struct typinfo TypInfo[] = {
122123
F_XIDIN, F_XIDOUT},
123124
{"cid", CIDOID, 0, 4, true, 'i', 'p',
124125
F_CIDIN, F_CIDOUT},
126+
{"pg_node_tree", PGNODETREEOID, 0, -1, false, 'i', 'x',
127+
F_PG_NODE_TREE_IN, F_PG_NODE_TREE_OUT},
125128
{"int2vector", INT2VECTOROID, INT2OID, -1, false, 'i', 'p',
126129
F_INT2VECTORIN, F_INT2VECTOROUT},
127130
{"oidvector", OIDVECTOROID, OIDOID, -1, false, 'i', 'p',

‎src/backend/utils/adt/pseudotypes.c

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
*
1717
*
1818
* IDENTIFICATION
19-
* $PostgreSQL: pgsql/src/backend/utils/adt/pseudotypes.c,v 1.23 2010/01/02 16:57:55 momjian Exp $
19+
* $PostgreSQL: pgsql/src/backend/utils/adt/pseudotypes.c,v 1.24 2010/09/03 01:34:55 tgl Exp $
2020
*
2121
*-------------------------------------------------------------------------
2222
*/
@@ -398,3 +398,58 @@ shell_out(PG_FUNCTION_ARGS)
398398

399399
PG_RETURN_VOID(); /* keep compiler quiet */
400400
}
401+
402+
403+
/*
404+
* pg_node_tree_in - input routine for type PG_NODE_TREE.
405+
*
406+
* pg_node_tree isn't really a pseudotype --- it's real enough to be a table
407+
* column --- but it presently has no operations of its own, and disallows
408+
* input too, so its I/O functions seem to fit here as much as anywhere.
409+
*/
410+
Datum
411+
pg_node_tree_in(PG_FUNCTION_ARGS)
412+
{
413+
/*
414+
* We disallow input of pg_node_tree values because the SQL functions that
415+
* operate on the type are not secure against malformed input.
416+
*/
417+
ereport(ERROR,
418+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
419+
errmsg("cannot accept a value of type pg_node_tree")));
420+
421+
PG_RETURN_VOID(); /* keep compiler quiet */
422+
}
423+
424+
/*
425+
* pg_node_tree_out - output routine for type PG_NODE_TREE.
426+
*
427+
* The internal representation is the same as TEXT, so just pass it off.
428+
*/
429+
Datum
430+
pg_node_tree_out(PG_FUNCTION_ARGS)
431+
{
432+
return textout(fcinfo);
433+
}
434+
435+
/*
436+
* pg_node_tree_recv - binary input routine for type PG_NODE_TREE.
437+
*/
438+
Datum
439+
pg_node_tree_recv(PG_FUNCTION_ARGS)
440+
{
441+
ereport(ERROR,
442+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
443+
errmsg("cannot accept a value of type pg_node_tree")));
444+
445+
PG_RETURN_VOID(); /* keep compiler quiet */
446+
}
447+
448+
/*
449+
* pg_node_tree_send - binary output routine for type PG_NODE_TREE.
450+
*/
451+
Datum
452+
pg_node_tree_send(PG_FUNCTION_ARGS)
453+
{
454+
return textsend(fcinfo);
455+
}

‎src/include/catalog/catversion.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
3838
* Portions Copyright (c) 1994, Regents of the University of California
3939
*
40-
* $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.597 2010/08/24 06:30:43 itagaki Exp $
40+
* $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.598 2010/09/03 01:34:55 tgl Exp $
4141
*
4242
*-------------------------------------------------------------------------
4343
*/
@@ -53,6 +53,6 @@
5353
*/
5454

5555
/* yyyymmddN */
56-
#define CATALOG_VERSION_NO 201008241
56+
#define CATALOG_VERSION_NO 201009021
5757

5858
#endif

‎src/include/catalog/genbki.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
1313
* Portions Copyright (c) 1994, Regents of the University of California
1414
*
15-
* $PostgreSQL: pgsql/src/include/catalog/genbki.h,v 1.6 2010/01/05 01:06:56 tgl Exp $
15+
* $PostgreSQL: pgsql/src/include/catalog/genbki.h,v 1.7 2010/09/03 01:34:55 tgl Exp $
1616
*
1717
*-------------------------------------------------------------------------
1818
*/
@@ -35,7 +35,8 @@
3535
#define DESCR(x) extern int no_such_variable
3636
#define SHDESCR(x) extern int no_such_variable
3737

38-
/* PHONY type definition for use in catalog structure definitions only */
38+
/* PHONY type definitions for use in catalog structure definitions only */
3939
typedef int aclitem;
40+
typedef int pg_node_tree;
4041

4142
#endif /* GENBKI_H */

‎src/include/catalog/pg_attrdef.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $PostgreSQL: pgsql/src/include/catalog/pg_attrdef.h,v 1.26 2010/01/05 01:06:56 tgl Exp $
11+
* $PostgreSQL: pgsql/src/include/catalog/pg_attrdef.h,v 1.27 2010/09/03 01:34:55 tgl Exp $
1212
*
1313
* NOTES
1414
* the genbki.pl script reads this file and generates .bki
@@ -30,10 +30,10 @@
3030

3131
CATALOG(pg_attrdef,2604)
3232
{
33-
Oid adrelid;
34-
int2 adnum;
35-
text adbin;
36-
text adsrc;
33+
Oid adrelid; /* OID of table containing attribute */
34+
int2 adnum; /* attnum of attribute */
35+
pg_node_tree adbin; /* nodeToString representation of default */
36+
text adsrc; /* human-readable representation of default */
3737
} FormData_pg_attrdef;
3838

3939
/* ----------------

‎src/include/catalog/pg_cast.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
* Copyright (c) 2002-2010, PostgreSQL Global Development Group
1212
*
13-
* $PostgreSQL: pgsql/src/include/catalog/pg_cast.h,v 1.45 2010/07/16 02:15:54 tgl Exp $
13+
* $PostgreSQL: pgsql/src/include/catalog/pg_cast.h,v 1.46 2010/09/03 01:34:55 tgl Exp $
1414
*
1515
* NOTES
1616
* the genbki.pl script reads this file and generates .bki
@@ -235,6 +235,9 @@ DATA(insert ( 1043 19 1400 i f ));
235235
DATA(insert ( 18 23 77 e f ));
236236
DATA(insert ( 23 18 78 e f ));
237237

238+
/* pg_node_tree can be coerced to, but not from, text */
239+
DATA(insert ( 194 25 0 i b ));
240+
238241
/*
239242
* Datetime category
240243
*/

‎src/include/catalog/pg_constraint.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $PostgreSQL: pgsql/src/include/catalog/pg_constraint.h,v 1.41 2010/08/07 02:44:07 tgl Exp $
11+
* $PostgreSQL: pgsql/src/include/catalog/pg_constraint.h,v 1.42 2010/09/03 01:34:55 tgl Exp $
1212
*
1313
* NOTES
1414
* the genbki.pl script reads this file and generates .bki
@@ -129,7 +129,7 @@ CATALOG(pg_constraint,2606)
129129
/*
130130
* If a check constraint, nodeToString representation of expression
131131
*/
132-
text conbin;
132+
pg_node_tree conbin;
133133

134134
/*
135135
* If a check constraint, source-text representation of expression

‎src/include/catalog/pg_index.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
99
* Portions Copyright (c) 1994, Regents of the University of California
1010
*
11-
* $PostgreSQL: pgsql/src/include/catalog/pg_index.h,v 1.50 2010/01/05 01:06:56 tgl Exp $
11+
* $PostgreSQL: pgsql/src/include/catalog/pg_index.h,v 1.51 2010/09/03 01:34:55 tgl Exp $
1212
*
1313
* NOTES
1414
* the genbki.pl script reads this file and generates .bki
@@ -45,10 +45,10 @@ CATALOG(pg_index,2610) BKI_WITHOUT_OIDS BKI_SCHEMA_MACRO
4545
int2vector indkey; /* column numbers of indexed cols, or 0 */
4646
oidvector indclass; /* opclass identifiers */
4747
int2vector indoption; /* per-column flags (AM-specific meanings) */
48-
text indexprs; /* expression trees for index attributes that
48+
pg_node_tree indexprs; /* expression trees for index attributes that
4949
* are not simple column references; one for
5050
* each zero entry in indkey[] */
51-
text indpred; /* expression tree for predicate, if a partial
51+
pg_node_tree indpred; /* expression tree for predicate, if a partial
5252
* index; else NULL */
5353
} FormData_pg_index;
5454

0 commit comments

Comments
 (0)