netxduo icon indicating copy to clipboard operation
netxduo copied to clipboard

Buffer overflow in nx_secure_tls_send_handshake_record()

Open ohoc opened this issue 2 years ago • 1 comments

Describe the bug During nx_web_http_client_secure_connect(), in function _nx_secure_tls_send_handshake_record(), NX_SECURE_MEMCPY overflows the nx_secure_tls_handshake_cache buffer, causing a crash later in the code.

image

image When NX_SECURE_MEMCPY is called, length==1117, whereas nx_secure_tls_handshake_cache is only 500 bytes long.

  • What target device are you using? STM32WB55
  • Which version of Azure RTOS? 6.2.0
  • What toolchain and environment? STM32CubeIDE 1.13.1
  • What have you tried to diagnose or workaround this issue? Yes, increasing nx_secure_tls_handshake_cache size seems to solve the issue, but it is not clear why the buffer is too small in the first place.

To Reproduce NX_SECURE_TLS_TLS_1_2_ENABLED=1 NX_SECURE_ENABLE_AEAD_CIPHER=1 NX_SECURE_TLS_SERVER_DISABLED=1 NX_SECURE_ALLOW_SELF_SIGNED_CERTIFICATES=1

tls_setup function:


static UINT tls_setup(NX_WEB_HTTP_CLIENT* client_ptr, NX_SECURE_TLS_SESSION* tls_session)
{
  UINT status;
  lfs_file_t fileCA;

  NX_PARAMETER_NOT_USED(client_ptr);

  https_tls_session = NULL;

  /* Initialize and create TLS session. */
  struct s_buffer* shared_buffer = params_buffers_get_usb_htts_shared_buffer();
  struct s_tls_buffers* tls_buffers = (struct s_tls_buffers*)shared_buffer->buffer;
  if (sizeof(tls_buffers) > shared_buffer->size) return NX_SIZE_ERROR;

  status = nx_secure_tls_session_create(tls_session, &nx_crypto_tls_ciphers_ecc, tls_buffers->metadata, sizeof(tls_buffers->metadata));

  /* Check status.  */
  if (status != NX_SUCCESS) {
    return (status);
  }

#if ((APP_ENVIRONMENT == NT_GTS_UAT) || (APP_ENVIRONMENT == NT_GTS_PROD))
  /* Initialize the server DNS name. */
  NX_SECURE_X509_DNS_NAME dns_name;
  status = nx_secure_x509_dns_name_initialize(&dns_name, "xyz.com", strlen("xyz.com")); // TODO: get from url
  log_trace("nx_secure_x509_dns_name_initialize: %d", status);

  /* Initialize SNI extension in previously-initialized TLS Session. */
  status = nx_secure_tls_session_sni_extension_set(tls_session, &dns_name);
  log_trace("nx_secure_tls_session_sni_extension_set: %d", status);
#endif

  /* Allocate space for packet reassembly. */
  status = nx_secure_tls_session_packet_buffer_set(tls_session, tls_buffers->packet_buffer, sizeof(tls_buffers->packet_buffer));
  /* Check status.  */
  if (status != NX_SUCCESS) {
    return (status);
  }

  /* Open and read the CA binary .der file from the flash based on the configured env  */
  status = lfs_file_open(flash_get_lfs_media(), &fileCA, FILENAME_CERT_AUTH, LFS_O_RDONLY);
  if (status < NX_SUCCESS) {
    log_error("Failed to open " FILENAME_CERT_AUTH" file");
    return (status);
  }

  status = lfs_file_read(flash_get_lfs_media(), &fileCA, test_ca_cert_der, sizeof(test_ca_cert_der));
  if (status <= NX_SUCCESS) {
    log_error("Failed to read " FILENAME_CERT_AUTH " file");
    return (status);
  } else {
    log_debug("Read %d bytes from " FILENAME_CERT_AUTH " file", status);
  }

  lfs_file_close(flash_get_lfs_media(), &fileCA);

  /* Add a CA Certificate to our trusted store for verifying incoming server certificates. */
  status = nx_secure_x509_certificate_initialize(&trusted_certificate, test_ca_cert_der, (USHORT)status, NX_NULL, 0, NULL, 0, NX_SECURE_X509_KEY_TYPE_NONE);
  if (status != NX_SUCCESS) {
    return (status);
  }

  status = nx_secure_tls_trusted_certificate_add(tls_session, &trusted_certificate);
  if (status != NX_SUCCESS) {
    return (status);
  }

  /* Need to allocate space for the certificate coming in from the remote host. */
  status = nx_secure_tls_remote_certificate_allocate(tls_session, &remote_certificate, remote_cert_buffer, sizeof(remote_cert_buffer));
  if (status != NX_SUCCESS) {
    return (status);
  }

  status = nx_secure_tls_remote_certificate_allocate(tls_session, &remote_issuer, remote_issuer_buffer, sizeof(remote_issuer_buffer));
  if (status != NX_SUCCESS) {
    return (status);
  }

  status = nx_secure_tls_ecc_initialize(tls_session, nx_crypto_ecc_supported_groups, nx_crypto_ecc_supported_groups_size, nx_crypto_ecc_curves);
  if (status != NX_SUCCESS) {
    return (status);
  }

  https_tls_session = tls_session;

  return (NX_SUCCESS);
}

Expected behavior N/A

Impact Crash

Logs and console output N/A

Additional context Add any other context about the problem here.

ohoc avatar Sep 21 '23 13:09 ohoc

Please move the dns_name variable to global as it will be used after the tls_setup() function.

yanwucai avatar Oct 09 '23 09:10 yanwucai