BUG/MINOR: payload: validate minimum keyshare_len in smp_fetch_ssl_keyshare_groups

The keyshare extension parsing loop reads dataPointer[readPosition+2]
and dataPointer[readPosition+3] inside the loop body, requiring at least
4 bytes to be safe. However, keyshare_len was only validated as >= 2.

With keyshare_len == 2 or 3, the first loop iteration would read past
the end of the extension data, causing an out-of-bounds read which is
harmless in practice. We also need to make sure that the read position
stops 4 bytes before the end in order to read the 4 next bytes.

This can be backported to stable versions.

diff --git a/src/payload.c b/src/payload.c
index d86f3b1dd2015337830e93aa61c9d55598a4ed9c..0221929b943cdcf9e755ef5d34b6795b795d7b85 100644 (file)
--- a/src/payload.c
+++ b/src/payload.c
@@ -697,13 +697,13 @@ smp_fetch_ssl_keyshare_groups(const struct arg *args, struct sample *smp, const
                                goto not_ssl_hello;
 
                        keyshare_len = (data[4] << 8) + data[5]; /* Client keyshare length */
-                       if (keyshare_len < 2 || keyshare_len > hs_len - 6)
-                               goto not_ssl_hello; /* at least 2 bytes per keyshare */
+                       if (keyshare_len < 4 || keyshare_len > hs_len - 6)
+                               goto not_ssl_hello; /* at least 4 bytes for one keyshare entry */
                        dataPointer = data + 6; /* start of keyshare entries */
                        readPosition = 0;
                        numberOfKeyshares = 0;
                        smp_trash = get_trash_chunk();
-                       while (readPosition < keyshare_len) {
+                       while (readPosition + 4 <= keyshare_len) {
                                /* Get the binary value of the keyshare group and move the offset to the end of the related keyshare */
                                memmove(b_orig(smp_trash) + (2*numberOfKeyshares), &dataPointer[readPosition], 2);
                                numberOfKeyshares++;