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

Commit8983852

Browse files
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 parente5be899 commit8983852

File tree

6 files changed

+116
-24
lines changed

6 files changed

+116
-24
lines changed

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

Lines 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
structTSQueryParserStateData
@@ -388,14 +389,14 @@ makepol(TSQueryParserState state,
388389
* QueryItems must be in polish (prefix) notation.
389390
*/
390391
staticvoid
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 beencleansed
399+
* they haven't beencleaned
399400
* away yet.
400401
*/
401402
{
@@ -451,7 +452,7 @@ parse_tsquery(char *buf,
451452
TSQueryquery;
452453
intcommonlen;
453454
QueryItem*ptr;
454-
intpos=0;
455+
uint32pos=0;
455456
ListCell*cell;
456457

457458
/* init state */
@@ -792,14 +793,15 @@ tsqueryrecv(PG_FUNCTION_ARGS)
792793
QueryItem*item;
793794
intdatalen=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+
uint32left= (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

Lines 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
staticvoid
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
staticvoid
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
staticNODE*
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
returnnode;
130139

@@ -203,6 +212,9 @@ clean_fakeval_intree(NODE * node, char *result)
203212
charlresult=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
returnnode;
208220
else

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

Lines 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
staticint
2323
addone(int*counters,intlast,inttotal)
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
staticQTNode*
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

Lines 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
inti;
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
inti;
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
inti;
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
staticvoid
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
staticvoid
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

Lines 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
inti;
509509
boolfound= 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

Lines 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
QueryItemTypetype;/* operand or kind of operator (ts_tokentype) */
162162
int8weight;/* 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
int32valcrc;/* 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
QueryItemTypetype;
184190
int8oper;/* see above */
185-
int16left;/* pointer to left operand. Right operand is
191+
uint32left;/* 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)
[8]ページ先頭
©2009-2025 Movatter.jp