BUG/MINOR: payload: fix the handshake length bounds check smp_client_hello_parse()

After reading the handshake length, which is covered by the previous
4 bytes check, the size was not subtracted before being compared to the
retrieved handshake length, making it possible to accept a handshake
that claims to be 4 bytes larger than it really is. Similarly, a few
lines later, data[34] is accessed without checking that it is present,
because the test is made on the second hs_len, which doesn't guarantee
that the data are there.

This fix adds both tests. It can be backported to all stable versions
as it was introduced in 1.6 with commit bb2acf589f ("MINOR: payload:
add support for tls session ticket ext").

diff --git a/src/payload.c b/src/payload.c
index 45754118946d2a64bee712ed036672b5d10f0062..5e39133ba993685c08d92e235309d46845477377 100644 (file)
--- a/src/payload.c
+++ b/src/payload.c
@@ -116,6 +116,9 @@ smp_client_hello_parse( struct sample *smp, enum client_hello_type type, unsigne
        data += 5; /* enter TLS handshake */
        bleft -= 5;
 
+       if (bleft < hs_len)
+               goto too_short;
+
        /* Check for a complete client hello starting at <data> */
        if (bleft < 1)
                goto too_short;
@@ -129,15 +132,18 @@ smp_client_hello_parse( struct sample *smp, enum client_hello_type type, unsigne
        if (hs_len < 2 + 32 + 1 + 2 + 2 + 1 + 1 + 2 + 2)
                goto not_ssl_hello; /* too short to have an extension */
 
+       data += 4;
+       bleft -= 4;
+
        /* We want the full handshake here */
        if (bleft < hs_len)
                goto too_short;
 
-       data += 4;
        /* Start of the ClientHello message */
        if (data[0] < 0x03 || data[1] < 0x01) /* TLSv1 minimum */
                goto not_ssl_hello;
 
+       /* Note: covered by the hs_len test 30 lines above */
        ext_len = data[34]; /* session_id_len */
        if (ext_len > 32 || ext_len > (hs_len - 35)) /* check for correct session_id len */
                goto not_ssl_hello;