Unverified Commit 9b5e12a5 authored by Jay Satiro's avatar Jay Satiro Committed by Daniel Stenberg
Browse files

url: fix alignment of ssl_backend_data struct 

- Align the array of ssl_backend_data on a max 32 byte boundary.

8 is likely to be ok but I went with 32 for posterity should one of
the ssl_backend_data structs change to contain a larger sized variable
in the future.

Prior to this change (since dev 70f1db32, release 7.56) the connectdata
structure was undersized by 4 bytes in 32-bit builds with ssl enabled
because long long * was mistakenly used for alignment instead of
long long, with the intention being an 8 byte boundary. Also long long
may not be an available type.

The undersized connectdata could lead to oob read/write past the end in
what was expected to be the last 4 bytes of the connection's secondary
socket https proxy ssl_backend_data struct (the secondary socket in a
connection is used by ftp, others?).

Closes https://github.com/curl/curl/issues/2093

CVE-2017-8818

Bug: https://curl.haxx.se/docs/adv_2017-af0a.html
parent c79b2ca0

lib/url.c

+30 −21
Original line number Diff line number Diff line
@@ -1793,15 +1793,41 @@ static void llist_dtor(void *user, void *element) 
 */

static struct connectdata *allocate_conn(struct Curl_easy *data)

{

  struct connectdata *conn;

  size_t connsize = sizeof(struct connectdata);



#ifdef USE_SSL

#define SSL_EXTRA + 4 * Curl_ssl->sizeof_ssl_backend_data - sizeof(long long)

#else

#define SSL_EXTRA 0

/* SSLBK_MAX_ALIGN: The max byte alignment a CPU would use */

#define SSLBK_MAX_ALIGN 32

  /* The SSL backend-specific data (ssl_backend_data) objects are allocated as

     part of connectdata at the end. To ensure suitable alignment we will

     assume a maximum of SSLBK_MAX_ALIGN for alignment. Since calloc returns a

     pointer suitably aligned for any variable this will ensure the

     ssl_backend_data array has proper alignment, even if that alignment turns

     out to be less than SSLBK_MAX_ALIGN. */

  size_t paddingsize = sizeof(struct connectdata) % SSLBK_MAX_ALIGN;

  size_t alignsize = paddingsize ? (SSLBK_MAX_ALIGN - paddingsize) : 0;

  size_t sslbksize = Curl_ssl->sizeof_ssl_backend_data;

  connsize += alignsize + (4 * sslbksize);

#endif

  struct connectdata *conn = calloc(1, sizeof(struct connectdata) + SSL_EXTRA);



  conn = calloc(1, connsize);

  if(!conn)

    return NULL;



#ifdef USE_SSL

  /* Point to the ssl_backend_data objects at the end of connectdata.

     Note that these backend pointers can be swapped by vtls (eg ssl backend

     data becomes proxy backend data). */

  {

    char *end = (char *)conn + connsize;

    conn->ssl[0].backend = ((void *)(end - (4 * sslbksize)));

    conn->ssl[1].backend = ((void *)(end - (3 * sslbksize)));

    conn->proxy_ssl[0].backend = ((void *)(end - (2 * sslbksize)));

    conn->proxy_ssl[1].backend = ((void *)(end - (1 * sslbksize)));

  }

#endif



  conn->handler = &Curl_handler_dummy;  /* Be sure we have a handler defined

                                           already from start to avoid NULL

                                           situations and checks */
@@ -1881,23 +1907,6 @@ static struct connectdata *allocate_conn(struct Curl_easy *data) 


  conn->ip_version = data->set.ipver;



#ifdef USE_SSL

  /*

   * To save on malloc()s, the SSL backend-specific data has been allocated

   * at the end of the connectdata struct.

   */

  {

    char *p = (char *)&conn->align_data__do_not_use;

    conn->ssl[0].backend = (struct ssl_backend_data *)p;

    conn->ssl[1].backend =

      (struct ssl_backend_data *)(p + Curl_ssl->sizeof_ssl_backend_data);

    conn->proxy_ssl[0].backend =

      (struct ssl_backend_data *)(p + Curl_ssl->sizeof_ssl_backend_data * 2);

    conn->proxy_ssl[1].backend =

      (struct ssl_backend_data *)(p + Curl_ssl->sizeof_ssl_backend_data * 3);

  }

#endif



#if !defined(CURL_DISABLE_HTTP) && defined(USE_NTLM) && \

    defined(NTLM_WB_ENABLED)

  conn->ntlm_auth_hlpr_socket = CURL_SOCKET_BAD;

lib/urldata.h

+0 −10
Original line number Diff line number Diff line
@@ -1004,16 +1004,6 @@ struct connectdata { 
  char *unix_domain_socket;

  bool abstract_unix_socket;

#endif



#ifdef USE_SSL

  /*

   * To avoid multiple malloc() calls, the ssl_connect_data structures

   * associated with a connectdata struct are allocated in the same block

   * as the latter. This field forces alignment to an 8-byte boundary so

   * that this all works.

   */

  long long *align_data__do_not_use;

#endif

};



/* The end of connectdata. */