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

Commit9626325

Browse files
committed
Add heuristic incoming-message-size limits in the server.
We had a report of confusing server behavior caused by a client bugthat sent junk to the server: the server thought the junk was avery long message length and waited patiently for data that wouldnever come. We can reduce the risk of that by being less trustingabout message lengths.For a long time, libpq has had a heuristic rule that it wouldn'tbelieve large message size words, except for a small number ofmessage types that are expected to be (potentially) long. Thisprovides some defense against loss of message-boundary sync andother corrupted-data cases. The server does something similar,except that up to now it only limited the lengths of messagesreceived during the connection authentication phase. Let'sdo the same as in libpq and put restrictions on the allowedlength of all messages, while distinguishing between messagetypes that are expected to be long and those that aren't.I used a limit of 10000 bytes for non-long messages. (libpq'scorresponding limit is 30000 bytes, but given the asymmetry ofthe FE/BE protocol, there's no good reason why the numbers shouldbe the same.) Experimentation suggests that this is at least afactor of 10, maybe a factor of 100, more than we really need;but plenty of daylight seems desirable to avoid false positives.In any case we can adjust the limit based on beta-test results.For long messages, set a limit of MaxAllocSize - 1, which is themost that we can absorb into the StringInfo buffer that the messageis collected in. This just serves to make sure that a bogus messagesize is reported as such, rather than as a confusing gripe aboutnot being able to enlarge a string buffer.While at it, make sure that non-mainline code paths (such asCOPY FROM STDIN) are as paranoid as SocketBackend is, and validatethe message type code before believing the message length.This provides an additional guard against getting stuck on corruptedinput.Discussion:https://postgr.es/m/2003757.1619373089@sss.pgh.pa.us
1 parentd6b8d29 commit9626325

File tree

6 files changed

+82
-19
lines changed

6 files changed

+82
-19
lines changed

‎src/backend/commands/copyfromparse.c

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ CopyGetData(CopyFromState cstate, void *databuf, int minread, int maxread)
265265
{
266266
/* Try to receive another message */
267267
intmtype;
268+
intmaxmsglen;
268269

269270
readmessage:
270271
HOLD_CANCEL_INTERRUPTS();
@@ -274,11 +275,33 @@ CopyGetData(CopyFromState cstate, void *databuf, int minread, int maxread)
274275
ereport(ERROR,
275276
(errcode(ERRCODE_CONNECTION_FAILURE),
276277
errmsg("unexpected EOF on client connection with an open transaction")));
277-
if (pq_getmessage(cstate->fe_msgbuf,0))
278+
/* Validate message type and set packet size limit */
279+
switch (mtype)
280+
{
281+
case'd':/* CopyData */
282+
maxmsglen=PQ_LARGE_MESSAGE_LIMIT;
283+
break;
284+
case'c':/* CopyDone */
285+
case'f':/* CopyFail */
286+
case'H':/* Flush */
287+
case'S':/* Sync */
288+
maxmsglen=PQ_SMALL_MESSAGE_LIMIT;
289+
break;
290+
default:
291+
ereport(ERROR,
292+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
293+
errmsg("unexpected message type 0x%02X during COPY from stdin",
294+
mtype)));
295+
maxmsglen=0;/* keep compiler quiet */
296+
break;
297+
}
298+
/* Now collect the message body */
299+
if (pq_getmessage(cstate->fe_msgbuf,maxmsglen))
278300
ereport(ERROR,
279301
(errcode(ERRCODE_CONNECTION_FAILURE),
280302
errmsg("unexpected EOF on client connection with an open transaction")));
281303
RESUME_CANCEL_INTERRUPTS();
304+
/* ... and process it */
282305
switch (mtype)
283306
{
284307
case'd':/* CopyData */
@@ -304,11 +327,7 @@ CopyGetData(CopyFromState cstate, void *databuf, int minread, int maxread)
304327
*/
305328
gotoreadmessage;
306329
default:
307-
ereport(ERROR,
308-
(errcode(ERRCODE_PROTOCOL_VIOLATION),
309-
errmsg("unexpected message type 0x%02X during COPY from stdin",
310-
mtype)));
311-
break;
330+
Assert(false);/* NOT REACHED */
312331
}
313332
}
314333
avail=cstate->fe_msgbuf->len-cstate->fe_msgbuf->cursor;

‎src/backend/libpq/auth.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ static intPerformRadiusTransaction(const char *server, const char *secret, cons
210210

211211
/*
212212
* Maximum accepted size of GSS and SSPI authentication tokens.
213+
* We also use this as a limit on ordinary password packet lengths.
213214
*
214215
* Kerberos tickets are usually quite small, but the TGTs issued by Windows
215216
* domain controllers include an authorization field known as the Privilege
@@ -724,7 +725,7 @@ recv_password_packet(Port *port)
724725
}
725726

726727
initStringInfo(&buf);
727-
if (pq_getmessage(&buf,0))/* receive password */
728+
if (pq_getmessage(&buf,PG_MAX_AUTH_TOKEN_LENGTH))/* receive password */
728729
{
729730
/* EOF - pq_getmessage already logged a suitable message */
730731
pfree(buf.data);

‎src/backend/libpq/pqcomm.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1203,7 +1203,7 @@ pq_is_reading_msg(void)
12031203
*is removed. Also, s->cursor is initialized to zero for convenience
12041204
*in scanning the message contents.
12051205
*
1206-
*Ifmaxlen isnot zero, it is an upper limit on the length of the
1206+
*maxlen isthe upper limit on the length of the
12071207
*message we are willing to accept. We abort the connection (by
12081208
*returning EOF) if client tries to send more than that.
12091209
*
@@ -1230,8 +1230,7 @@ pq_getmessage(StringInfo s, int maxlen)
12301230

12311231
len=pg_ntoh32(len);
12321232

1233-
if (len<4||
1234-
(maxlen>0&&len>maxlen))
1233+
if (len<4||len>maxlen)
12351234
{
12361235
ereport(COMMERROR,
12371236
(errcode(ERRCODE_PROTOCOL_VIOLATION),

‎src/backend/replication/walsender.c

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1704,6 +1704,7 @@ static void
17041704
ProcessRepliesIfAny(void)
17051705
{
17061706
unsignedcharfirstchar;
1707+
intmaxmsglen;
17071708
intr;
17081709
boolreceived= false;
17091710

@@ -1733,17 +1734,36 @@ ProcessRepliesIfAny(void)
17331734
break;
17341735
}
17351736

1737+
/* Validate message type and set packet size limit */
1738+
switch (firstchar)
1739+
{
1740+
case'd':
1741+
maxmsglen=PQ_LARGE_MESSAGE_LIMIT;
1742+
break;
1743+
case'c':
1744+
case'X':
1745+
maxmsglen=PQ_SMALL_MESSAGE_LIMIT;
1746+
break;
1747+
default:
1748+
ereport(FATAL,
1749+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
1750+
errmsg("invalid standby message type \"%c\"",
1751+
firstchar)));
1752+
maxmsglen=0;/* keep compiler quiet */
1753+
break;
1754+
}
1755+
17361756
/* Read the message contents */
17371757
resetStringInfo(&reply_message);
1738-
if (pq_getmessage(&reply_message,0))
1758+
if (pq_getmessage(&reply_message,maxmsglen))
17391759
{
17401760
ereport(COMMERROR,
17411761
(errcode(ERRCODE_PROTOCOL_VIOLATION),
17421762
errmsg("unexpected EOF on standby connection")));
17431763
proc_exit(0);
17441764
}
17451765

1746-
/*Handle the very limited subset of commands expected in this phase */
1766+
/*... and process it */
17471767
switch (firstchar)
17481768
{
17491769
/*
@@ -1776,10 +1796,7 @@ ProcessRepliesIfAny(void)
17761796
proc_exit(0);
17771797

17781798
default:
1779-
ereport(FATAL,
1780-
(errcode(ERRCODE_PROTOCOL_VIOLATION),
1781-
errmsg("invalid standby message type \"%c\"",
1782-
firstchar)));
1799+
Assert(false);/* NOT REACHED */
17831800
}
17841801
}
17851802

‎src/backend/tcop/postgres.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,7 @@ static int
343343
SocketBackend(StringInfoinBuf)
344344
{
345345
intqtype;
346+
intmaxmsglen;
346347

347348
/*
348349
* Get message type code from the frontend.
@@ -375,45 +376,61 @@ SocketBackend(StringInfo inBuf)
375376
/*
376377
* Validate message type code before trying to read body; if we have lost
377378
* sync, better to say "command unknown" than to run out of memory because
378-
* we used garbage as a length word.
379+
* we used garbage as a length word. We can also select a type-dependent
380+
* limit on what a sane length word could be. (The limit could be chosen
381+
* more granularly, but it's not clear it's worth fussing over.)
379382
*
380383
* This also gives us a place to set the doing_extended_query_message flag
381384
* as soon as possible.
382385
*/
383386
switch (qtype)
384387
{
385388
case'Q':/* simple query */
389+
maxmsglen=PQ_LARGE_MESSAGE_LIMIT;
386390
doing_extended_query_message= false;
387391
break;
388392

389393
case'F':/* fastpath function call */
394+
maxmsglen=PQ_LARGE_MESSAGE_LIMIT;
390395
doing_extended_query_message= false;
391396
break;
392397

393398
case'X':/* terminate */
399+
maxmsglen=PQ_SMALL_MESSAGE_LIMIT;
394400
doing_extended_query_message= false;
395401
ignore_till_sync= false;
396402
break;
397403

398404
case'B':/* bind */
405+
case'P':/* parse */
406+
maxmsglen=PQ_LARGE_MESSAGE_LIMIT;
407+
doing_extended_query_message= true;
408+
break;
409+
399410
case'C':/* close */
400411
case'D':/* describe */
401412
case'E':/* execute */
402413
case'H':/* flush */
403-
case'P':/* parse */
414+
maxmsglen=PQ_SMALL_MESSAGE_LIMIT;
404415
doing_extended_query_message= true;
405416
break;
406417

407418
case'S':/* sync */
419+
maxmsglen=PQ_SMALL_MESSAGE_LIMIT;
408420
/* stop any active skip-till-Sync */
409421
ignore_till_sync= false;
410422
/* mark not-extended, so that a new error doesn't begin skip */
411423
doing_extended_query_message= false;
412424
break;
413425

414426
case'd':/* copy data */
427+
maxmsglen=PQ_LARGE_MESSAGE_LIMIT;
428+
doing_extended_query_message= false;
429+
break;
430+
415431
case'c':/* copy done */
416432
case'f':/* copy fail */
433+
maxmsglen=PQ_SMALL_MESSAGE_LIMIT;
417434
doing_extended_query_message= false;
418435
break;
419436

@@ -427,6 +444,7 @@ SocketBackend(StringInfo inBuf)
427444
ereport(FATAL,
428445
(errcode(ERRCODE_PROTOCOL_VIOLATION),
429446
errmsg("invalid frontend message type %d",qtype)));
447+
maxmsglen=0;/* keep compiler quiet */
430448
break;
431449
}
432450

@@ -435,7 +453,7 @@ SocketBackend(StringInfo inBuf)
435453
* after the type code; we can read the message contents independently of
436454
* the type.
437455
*/
438-
if (pq_getmessage(inBuf,0))
456+
if (pq_getmessage(inBuf,maxmsglen))
439457
returnEOF;/* suitable message already logged */
440458
RESUME_CANCEL_INTERRUPTS();
441459

‎src/include/libpq/libpq.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@
2121
#include"storage/latch.h"
2222

2323

24+
/*
25+
* Callers of pq_getmessage() must supply a maximum expected message size.
26+
* By convention, if there's not any specific reason to use another value,
27+
* use PQ_SMALL_MESSAGE_LIMIT for messages that shouldn't be too long, and
28+
* PQ_LARGE_MESSAGE_LIMIT for messages that can be long.
29+
*/
30+
#definePQ_SMALL_MESSAGE_LIMIT10000
31+
#definePQ_LARGE_MESSAGE_LIMIT(MaxAllocSize - 1)
32+
2433
typedefstruct
2534
{
2635
void(*comm_reset) (void);

0 commit comments

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