Fix SCRAM auth handling bug.
authorTatsuo Ishii <ishii@sraoss.co.jp>
Thu, 12 Mar 2020 06:49:35 +0000 (15:49 +0900)
committerTatsuo Ishii <ishii@sraoss.co.jp>
Fri, 13 Mar 2020 00:31:58 +0000 (09:31 +0900)
Comment on the patch from the author:

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.

Bug reported in:
https://www.pgpool.net/mantisbt/view.php?id=595

Patch author:
Greg Nancarrow (Fujitsu Australia)

src/auth/pool_auth.c

index ba6d1e7b6c1c57e6f772f66367c133b4ae94403e..b6a47d08b331acccfe6acaeea3b148f72d0e8b87 100644 (file)
@@ -2369,7 +2369,7 @@ do_SCRAM(POOL_CONNECTION * frontend, POOL_CONNECTION * backend, int protoMajor,
                                ereport(ERROR,
                                                (errmsg("invalid authentication request from server: unknown auth kind %d", auth_kind)));
                }
-               /* Read next packend */
+               /* Read next backend */
                pool_read(backend, &kind, sizeof(kind));
                pool_read(backend, &len, sizeof(len));
                if (kind != 'R')
@@ -2377,7 +2377,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 ")));