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
Appearance settings

Commit 8983852

Browse filesBrowse files
feodorfeodor
committed
Improving various checks by Heikki Linnakangas <heikki@enterprisedb.com>
- add code to check that the query tree is well-formed. It was indeed possible to send malformed queries in binary mode, which produced all kinds of strange results. - make the left-field a uint32. There's no reason to arbitrarily limit it to 16-bits, and it won't increase the disk/memory footprint either now that QueryOperator and QueryOperand are separate structs. - add check_stack_depth() call to all recursive functions I found. Some of them might have a natural limit so that you can't force arbitrarily deep recursions, but check_stack_depth() is cheap enough that seems best to just stick it into anything that might be a problem.
1 parent e5be899 commit 8983852
Copy full SHA for 8983852

File tree

Expand file treeCollapse file tree

6 files changed

+116
-24
lines changed
Filter options
Expand file treeCollapse file tree

6 files changed

+116
-24
lines changed

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

Copy file name to clipboardExpand all lines: src/backend/utils/adt/tsquery.c
+51-16Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery.c,v 1.3 2007/09/07 15:09:56 teodor Exp $
10+
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery.c,v 1.4 2007/09/07 15:35:10 teodor Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -21,6 +21,7 @@
2121
#include "tsearch/ts_utils.h"
2222
#include "utils/memutils.h"
2323
#include "utils/pg_crc.h"
24+
#include "nodes/bitmapset.h"
2425

2526

2627
struct TSQueryParserStateData
@@ -388,14 +389,14 @@ makepol(TSQueryParserState state,
388389
* QueryItems must be in polish (prefix) notation.
389390
*/
390391
static void
391-
findoprnd(QueryItem *ptr, int *pos)
392+
findoprnd(QueryItem *ptr, uint32 *pos)
392393
{
393394
/* since this function recurses, it could be driven to stack overflow. */
394395
check_stack_depth();
395396

396397
if (ptr[*pos].type == QI_VAL ||
397398
ptr[*pos].type == QI_VALSTOP) /* need to handle VALSTOP here,
398-
* they haven't been cleansed
399+
* they haven't been cleaned
399400
* away yet.
400401
*/
401402
{
@@ -451,7 +452,7 @@ parse_tsquery(char *buf,
451452
TSQuery query;
452453
int commonlen;
453454
QueryItem *ptr;
454-
int pos = 0;
455+
uint32 pos = 0;
455456
ListCell *cell;
456457

457458
/* init state */
@@ -792,14 +793,15 @@ tsqueryrecv(PG_FUNCTION_ARGS)
792793
QueryItem *item;
793794
int datalen = 0;
794795
char *ptr;
796+
Bitmapset *parentset = NULL;
795797

796798
size = pq_getmsgint(buf, sizeof(uint32));
797799
if (size < 0 || size > (MaxAllocSize / sizeof(QueryItem)))
798800
elog(ERROR, "invalid size of tsquery");
799801

800802
len = HDRSIZETQ + sizeof(QueryItem) * size;
801803

802-
query = (TSQuery) palloc(len);
804+
query = (TSQuery) palloc0(len);
803805
query->size = size;
804806
item = GETQUERY(query);
805807

@@ -814,6 +816,15 @@ tsqueryrecv(PG_FUNCTION_ARGS)
814816
item->operand.valcrc = (int32) pq_getmsgint(buf, sizeof(int32));
815817
item->operand.length = pq_getmsgint(buf, sizeof(int16));
816818

819+
/* Check that the weight bitmap is valid */
820+
if (item->operand.weight < 0 || item->operand.weight > 0xF)
821+
elog(ERROR, "invalid weight bitmap");
822+
823+
/* XXX: We don't check that the CRC is valid. Actually, if we
824+
* bothered to calculate it to verify, there would be no need
825+
* to transfer it.
826+
*/
827+
817828
/*
818829
* Check that datalen doesn't grow too large. Without the
819830
* check, a malicious client could induce a buffer overflow
@@ -828,7 +839,7 @@ tsqueryrecv(PG_FUNCTION_ARGS)
828839
elog(ERROR, "invalid tsquery; total operand length exceeded");
829840

830841
/* We can calculate distance from datalen, no need to send it
831-
* through the wire. If we did, we would have to check that
842+
* across the wire. If we did, we would have to check that
832843
* it's valid anyway.
833844
*/
834845
item->operand.distance = datalen;
@@ -842,24 +853,41 @@ tsqueryrecv(PG_FUNCTION_ARGS)
842853
item->operator.oper != OP_OR &&
843854
item->operator.oper != OP_AND)
844855
elog(ERROR, "unknown operator type %d", (int) item->operator.oper);
856+
857+
/*
858+
* Check that no previous operator node points to the right
859+
* operand. That would mean that the operand node
860+
* has two parents.
861+
*/
862+
if (bms_is_member(i + 1, parentset))
863+
elog(ERROR, "malformed query tree");
864+
865+
parentset = bms_add_member(parentset, i + 1);
866+
845867
if(item->operator.oper != OP_NOT)
846868
{
847-
item->operator.left = (int16) pq_getmsgint(buf, sizeof(int16));
869+
uint32 left = (uint32) pq_getmsgint(buf, sizeof(uint32));
870+
848871
/*
849-
* Sanity checks
872+
* Right operand is implicitly at "this+1". Don't allow
873+
* left to point to the right operand, or to self.
850874
*/
851-
if (item->operator.left <= 0 || i + item->operator.left >= size)
875+
if (left <= 1 || i + left >= size)
852876
elog(ERROR, "invalid pointer to left operand");
853877

854-
/* XXX: Though there's no way to construct a TSQuery that's
855-
* not in polish notation, we don't enforce that for
856-
* queries received from client in binary mode. Is there
857-
* anything that relies on it?
858-
*
859-
* XXX: The tree could be malformed in other ways too,
860-
* a node could have two parents, for example.
878+
/*
879+
* Check that no previous operator node points to the left
880+
* operand.
861881
*/
882+
if (bms_is_member(i + left, parentset))
883+
elog(ERROR, "malformed query tree");
884+
885+
parentset = bms_add_member(parentset, i + left);
886+
887+
item->operator.left = left;
862888
}
889+
else
890+
item->operator.left = 1; /* do not leave uninitialized fields */
863891

864892
if (i == size - 1)
865893
elog(ERROR, "invalid pointer to right operand");
@@ -871,6 +899,13 @@ tsqueryrecv(PG_FUNCTION_ARGS)
871899
item++;
872900
}
873901

902+
/* Now check that each node, except the root, has a parent. We
903+
* already checked above that no node has more than one parent. */
904+
if (bms_num_members(parentset) != size - 1 && size != 0)
905+
elog(ERROR, "malformed query tree");
906+
907+
bms_free( parentset );
908+
874909
query = (TSQuery) repalloc(query, len + datalen);
875910

876911
item = GETQUERY(query);

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

Copy file name to clipboardExpand all lines: src/backend/utils/adt/tsquery_cleanup.c
+13-1Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery_cleanup.c,v 1.2 2007/09/07 15:09:56 teodor Exp $
11+
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery_cleanup.c,v 1.3 2007/09/07 15:35:10 teodor Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -57,6 +57,9 @@ typedef struct
5757
static void
5858
plainnode(PLAINTREE * state, NODE * node)
5959
{
60+
/* since this function recurses, it could be driven to stack overflow. */
61+
check_stack_depth();
62+
6063
if (state->cur == state->len)
6164
{
6265
state->len *= 2;
@@ -107,6 +110,9 @@ plaintree(NODE * root, int *len)
107110
static void
108111
freetree(NODE * node)
109112
{
113+
/* since this function recurses, it could be driven to stack overflow. */
114+
check_stack_depth();
115+
110116
if (!node)
111117
return;
112118
if (node->left)
@@ -125,6 +131,9 @@ freetree(NODE * node)
125131
static NODE *
126132
clean_NOT_intree(NODE * node)
127133
{
134+
/* since this function recurses, it could be driven to stack overflow. */
135+
check_stack_depth();
136+
128137
if (node->valnode->type == QI_VAL)
129138
return node;
130139

@@ -203,6 +212,9 @@ clean_fakeval_intree(NODE * node, char *result)
203212
char lresult = V_UNKNOWN,
204213
rresult = V_UNKNOWN;
205214

215+
/* since this function recurses, it could be driven to stack overflow. */
216+
check_stack_depth();
217+
206218
if (node->valnode->type == QI_VAL)
207219
return node;
208220
else

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

Copy file name to clipboardExpand all lines: src/backend/utils/adt/tsquery_rewrite.c
+7-1Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery_rewrite.c,v 1.2 2007/09/07 15:09:56 teodor Exp $
10+
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery_rewrite.c,v 1.3 2007/09/07 15:35:10 teodor Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -22,6 +22,9 @@
2222
static int
2323
addone(int *counters, int last, int total)
2424
{
25+
/* since this function recurses, it could be driven to stack overflow. */
26+
check_stack_depth();
27+
2528
counters[last]++;
2629
if (counters[last] >= total)
2730
{
@@ -173,6 +176,9 @@ findeq(QTNode *node, QTNode *ex, QTNode *subs, bool *isfind)
173176
static QTNode *
174177
dofindsubquery(QTNode *root, QTNode *ex, QTNode *subs, bool *isfind)
175178
{
179+
/* since this function recurses, it could be driven to stack overflow. */
180+
check_stack_depth();
181+
176182
root = findeq(root, ex, subs, isfind);
177183

178184
if (root && (root->flags & QTN_NOCHANGE) == 0 && root->valnode->type == QI_OPR)

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

Copy file name to clipboardExpand all lines: src/backend/utils/adt/tsquery_util.c
+31-2Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery_util.c,v 1.2 2007/09/07 15:09:56 teodor Exp $
10+
* $PostgreSQL: pgsql/src/backend/utils/adt/tsquery_util.c,v 1.3 2007/09/07 15:35:10 teodor Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -22,6 +22,9 @@ QT2QTN(QueryItem * in, char *operand)
2222
{
2323
QTNode *node = (QTNode *) palloc0(sizeof(QTNode));
2424

25+
/* since this function recurses, it could be driven to stack overflow. */
26+
check_stack_depth();
27+
2528
node->valnode = in;
2629

2730
if (in->type == QI_OPR)
@@ -53,6 +56,9 @@ QTNFree(QTNode * in)
5356
if (!in)
5457
return;
5558

59+
/* since this function recurses, it could be driven to stack overflow. */
60+
check_stack_depth();
61+
5662
if (in->valnode->type == QI_VAL && in->word && (in->flags & QTN_WORDFREE) != 0)
5763
pfree(in->word);
5864

@@ -79,6 +85,9 @@ QTNFree(QTNode * in)
7985
int
8086
QTNodeCompare(QTNode * an, QTNode * bn)
8187
{
88+
/* since this function recurses, it could be driven to stack overflow. */
89+
check_stack_depth();
90+
8291
if (an->valnode->type != bn->valnode->type)
8392
return (an->valnode->type > bn->valnode->type) ? -1 : 1;
8493

@@ -133,6 +142,9 @@ QTNSort(QTNode * in)
133142
{
134143
int i;
135144

145+
/* since this function recurses, it could be driven to stack overflow. */
146+
check_stack_depth();
147+
136148
if (in->valnode->type != QI_OPR)
137149
return;
138150

@@ -165,6 +177,9 @@ QTNTernary(QTNode * in)
165177
{
166178
int i;
167179

180+
/* since this function recurses, it could be driven to stack overflow. */
181+
check_stack_depth();
182+
168183
if (in->valnode->type != QI_OPR)
169184
return;
170185

@@ -205,6 +220,9 @@ QTNBinary(QTNode * in)
205220
{
206221
int i;
207222

223+
/* since this function recurses, it could be driven to stack overflow. */
224+
check_stack_depth();
225+
208226
if (in->valnode->type != QI_OPR)
209227
return;
210228

@@ -244,6 +262,9 @@ QTNBinary(QTNode * in)
244262
static void
245263
cntsize(QTNode * in, int *sumlen, int *nnode)
246264
{
265+
/* since this function recurses, it could be driven to stack overflow. */
266+
check_stack_depth();
267+
247268
*nnode += 1;
248269
if (in->valnode->type == QI_OPR)
249270
{
@@ -268,6 +289,9 @@ typedef struct
268289
static void
269290
fillQT(QTN2QTState *state, QTNode *in)
270291
{
292+
/* since this function recurses, it could be driven to stack overflow. */
293+
check_stack_depth();
294+
271295
if (in->valnode->type == QI_VAL)
272296
{
273297
memcpy(state->curitem, in->valnode, sizeof(QueryOperand));
@@ -325,7 +349,12 @@ QTN2QT(QTNode *in)
325349
QTNode *
326350
QTNCopy(QTNode *in)
327351
{
328-
QTNode *out = (QTNode *) palloc(sizeof(QTNode));
352+
QTNode *out;
353+
354+
/* since this function recurses, it could be driven to stack overflow. */
355+
check_stack_depth();
356+
357+
out = (QTNode *) palloc(sizeof(QTNode));
329358

330359
*out = *in;
331360
out->valnode = (QueryItem *) palloc(sizeof(QueryItem));

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

Copy file name to clipboardExpand all lines: src/backend/utils/adt/tsrank.c
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
*
88
*
99
* IDENTIFICATION
10-
* $PostgreSQL: pgsql/src/backend/utils/adt/tsrank.c,v 1.2 2007/09/07 15:09:56 teodor Exp $
10+
* $PostgreSQL: pgsql/src/backend/utils/adt/tsrank.c,v 1.3 2007/09/07 15:35:10 teodor Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -508,6 +508,10 @@ Cover(DocRepresentation *doc, int len, TSQuery query, Extention *ext)
508508
int i;
509509
bool found = false;
510510

511+
/* since this function recurses, it could be driven to stack overflow.
512+
* (though any decent compiler will optimize away the tail-recursion. */
513+
check_stack_depth();
514+
511515
reset_istrue_flag(query);
512516

513517
ext->p = 0x7fffffff;

‎src/include/tsearch/ts_type.h

Copy file name to clipboardExpand all lines: src/include/tsearch/ts_type.h
+9-3Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
*
66
* Copyright (c) 1998-2007, PostgreSQL Global Development Group
77
*
8-
* $PostgreSQL: pgsql/src/include/tsearch/ts_type.h,v 1.2 2007/09/07 15:09:56 teodor Exp $
8+
* $PostgreSQL: pgsql/src/include/tsearch/ts_type.h,v 1.3 2007/09/07 15:35:11 teodor Exp $
99
*
1010
*-------------------------------------------------------------------------
1111
*/
@@ -160,7 +160,13 @@ typedef struct
160160
{
161161
QueryItemType type; /* operand or kind of operator (ts_tokentype) */
162162
int8 weight; /* weights of operand to search. It's a bitmask of allowed weights.
163-
* if it =0 then any weight are allowed */
163+
* if it =0 then any weight are allowed.
164+
* Weights and bit map:
165+
* A: 1<<3
166+
* B: 1<<2
167+
* C: 1<<1
168+
* D: 1<<0
169+
*/
164170
int32 valcrc; /* XXX: pg_crc32 would be a more appropriate data type,
165171
* but we use comparisons to signed integers in the code.
166172
* They would need to be changed as well. */
@@ -182,7 +188,7 @@ typedef struct
182188
{
183189
QueryItemType type;
184190
int8 oper; /* see above */
185-
int16 left; /* pointer to left operand. Right operand is
191+
uint32 left; /* pointer to left operand. Right operand is
186192
* item + 1, left operand is placed
187193
* item+item->left */
188194
} QueryOperator;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.