Re: [PATCH] Memory corruption building Proxy Protocol V2 header

[Dave McCowan]

Here is the corrected (and retested) :-) patch.

Commit comment: Use temporary trash chunk, instead of global trash chunk in
make_proxy_line_v2() to avoid memory overwrite.

--Dave

On Thu, Jul 17, 2014 at 4:17 AM, Willy Tarreau <w...@1wt.eu> wrote:

> On Wed, Jul 16, 2014 at 11:50:30PM -0400, Dave McCowan wrote:
> > Hi Willy,
> >
> > <blush>  Yes, I changed my variable names after testing to clean up and
> > failed.
> > Is my obvious corrected patch the correct fix?
>
> Yes I think so.
>
> > Or should we clamp down on the use of global chunks being passed
> downstream?
>
> Don't try to fix everything around, I find your fix the most durable one.
> Please simply resend it fixed, it's easier for me to track changes, and for
> anyone else to review and comment if needed.
>
> Thanks,
> Willy
>
>
>

diff --git a/src/connection.c b/src/connection.c
index 20a911b..3435b1a 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -622,6 +622,7 @@ int make_proxy_line_v2(char *buf, int buf_len, struct 
server *srv, struct connec
        char *value = NULL;
        struct tlv_ssl *tlv;
        int ssl_tlv_len = 0;
+       struct chunk *cn_trash;
 #endif
 
        if (buf_len < PP2_HEADER_LEN)
@@ -682,8 +683,9 @@ int make_proxy_line_v2(char *buf, int buf_len, struct 
server *srv, struct connec
                                tlv->verify = 
htonl(ssl_sock_get_verify_result(remote));
                        }
                        if (srv->pp_opts & SRV_PP_V2_SSL_CN) {
-                               if (ssl_sock_get_remote_common_name(remote, 
&trash) > 0) {
-                                       tlv_len = 
make_tlv(&buf[ret+ssl_tlv_len], (buf_len - ret - ssl_tlv_len), PP2_TYPE_SSL_CN, 
trash.len, trash.str);
+                               cn_trash = get_trash_chunk();
+                               if (ssl_sock_get_remote_common_name(remote, 
&cn_trash) > 0) {
+                                       tlv_len = 
make_tlv(&buf[ret+ssl_tlv_len], (buf_len - ret - ssl_tlv_len), PP2_TYPE_SSL_CN, 
cn_trash->len, cn_trash->str);
                                        ssl_tlv_len += tlv_len;
                                }
                        }