View Issue Details

IDProjectCategoryView StatusLast Update
0000595Pgpool-IIBugpublic2020-05-21 11:28
Reportergregn123Assigned Tot-ishii 
PrioritynormalSeverityminorReproducibilityN/A
Status closedResolutionopen 
Product Version4.1.0 
Target Version4.1.2Fixed in Version4.1.2 
Summary0000595: Incorrect message length validation in do_SCRAM() - backend authentication
DescriptionThere is an incorrect message length validation performed during backend authentication in the do_SCRAM() function (src/auth/pool_auth.c).

               /* Read next packend */
                pool_read(backend, &kind, sizeof(kind));
                pool_read(backend, &len, sizeof(len));
                if (kind != 'R')
                        ereport(ERROR,
                                        (errmsg("backend authentication failed"),
                                         errdetail("backend response with kind \'%c\' when expecting \'R\'", kind)));
                message_length = ntohl(len);
                if (len <= 8)
                        ereport(ERROR,
                                        (errmsg("backend authentication failed"),
                                         errdetail("backend response with no data ")));


The code is currently checking if "len <= 8", but len is is network-byte-order (big-endian).
It is surely meant to be checking "message_length" instead, which is "len" coverted to host-byte-order (see previous line of code).
Under (Intel) Linux, which is little-endian, the value of "len" will be a large number and thus render the current error condition check ineffective [for example, in one case that I debugged, an example value of len was 134217728 (0x08000000), meaning that message_length was actually 8].
Additionally, it seems the "<=" check should actually be "<", based on the length values that I see when debugging this code.
I've attached a proposed patch (pgpool2_pool_auth_do_scram.patch).
Please review.
TagsNo tags attached.

Activities

gregn123

2020-03-12 12:18

reporter  

pgpool2_pool_auth_do_scram.patch (572 bytes)
diff --git a/src/auth/pool_auth.c b/src/auth/pool_auth.c
index 3421c17..0f6bf54 100644
--- a/src/auth/pool_auth.c
+++ b/src/auth/pool_auth.c
@@ -2386,7 +2386,7 @@ do_SCRAM(POOL_CONNECTION * frontend, POOL_CONNECTION * backend, int protoMajor,
 					(errmsg("backend authentication failed"),
 					 errdetail("backend response with kind \'%c\' when expecting \'R\'", kind)));
 		message_length = ntohl(len);
-		if (len <= 8)
+		if (message_length < 8)
 			ereport(ERROR,
 					(errmsg("backend authentication failed"),
 					 errdetail("backend response with no data ")));

t-ishii

2020-03-12 15:35

developer   ~0003262

Good catch! I will take care of the patch.

t-ishii

2020-03-12 15:52

developer   ~0003263

Your patch looks good. Can you please share your name (or email etc.) to be credited for the patch?

gregn123

2020-03-13 09:24

reporter   ~0003266

Greg Nancarrow (Fujitsu Australia)

t-ishii

2020-03-13 09:34

developer   ~0003267

Patch pushed. Thanks!

Issue History

Date Modified Username Field Change
2020-03-12 12:18 gregn123 New Issue
2020-03-12 12:18 gregn123 File Added: pgpool2_pool_auth_do_scram.patch
2020-03-12 15:35 t-ishii Assigned To => t-ishii
2020-03-12 15:35 t-ishii Status new => assigned
2020-03-12 15:35 t-ishii Note Added: 0003262
2020-03-12 15:52 t-ishii Note Added: 0003263
2020-03-13 08:52 t-ishii Status assigned => feedback
2020-03-13 08:52 t-ishii Description Updated View Revisions
2020-03-13 09:24 gregn123 Note Added: 0003266
2020-03-13 09:24 gregn123 Status feedback => assigned
2020-03-13 09:34 t-ishii Note Added: 0003267
2020-03-13 09:35 t-ishii Target Version => 4.1.2
2020-03-13 17:27 t-ishii Status assigned => feedback
2020-03-20 17:49 t-ishii Status feedback => resolved
2020-05-21 11:27 administrator Fixed in Version => 4.1.2
2020-05-21 11:28 administrator Status resolved => closed