Segfault with empty String domain

[DaanA32]

A segfault occurs in the following (code adapted from example/google-connect.rs):

It does work when the String is not empty.

extern crate native_tls;

use native_tls::TlsConnector;
use std::io::{Read, Write};
use std::net::TcpStream;

fn main() {
    let connector = TlsConnector::builder()
        .use_sni(false)
        .build()
        .unwrap();
    let domain = String::from("");
    // if domain is &'static str it works
    let stream = TcpStream::connect("google.com:443").unwrap();
    let mut stream = connector.connect(domain.as_str(), stream).unwrap();

    stream.write_all(b"GET / HTTP/1.0\r\n\r\n").unwrap();
    let mut res = vec![];
    stream.read_to_end(&mut res).unwrap();
    println!("{}", String::from_utf8_lossy(&res));
}

Output:

    Finished dev [unoptimized + debuginfo] target(s) in 0.39s
     Running `target/debug/examples/google-segfault`
[1]    136936 segmentation fault (core dumped)  cargo run --example google-segfault

Valgrind log

==135192== Memcheck, a memory error detector
==135192== Copyright (C) 2002-2022, and GNU GPL'd, by Julian Seward et al.
==135192== Using Valgrind-3.21.0 and LibVEX; rerun with -h for copyright info
==135192== Command: target/debug/examples/google-segfault
==135192== 
==135192== Invalid read of size 1
==135192==    at 0x4847EC6: strlen (vg_replace_strmem.c:501)
==135192==    by 0x4B74C85: int_x509_param_set_hosts (x509_vpm.c:46)
==135192==    by 0x12CD5B: openssl::x509::verify::X509VerifyParamRef::set_host (verify.rs:123)
==135192==    by 0x12F25E: openssl::ssl::connector::setup_verify_hostname (connector.rs:400)
==135192==    by 0x12F043: openssl::ssl::connector::ConnectConfiguration::into_ssl (connector.rs:188)
==135192==    by 0x11F610: openssl::ssl::connector::ConnectConfiguration::connect (connector.rs:201)
==135192==    by 0x11A15A: native_tls::imp::TlsConnector::connect (openssl.rs:344)
==135192==    by 0x11D90A: native_tls::TlsConnector::connect (lib.rs:514)
==135192==    by 0x11A981: google_segfault::main (google-segfault.rs:15)
==135192==    by 0x11667A: core::ops::function::FnOnce::call_once (function.rs:248)
==135192==    by 0x11E7BD: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:122)
==135192==    by 0x11DFE0: std::rt::lang_start::{{closure}} (rt.rs:166)
==135192==  Address 0x1 is not stack'd, malloc'd or (recently) free'd
==135192== 
==135192== 
==135192== Process terminating with default action of signal 11 (SIGSEGV): dumping core
==135192==  Access not within mapped region at address 0x1
==135192==    at 0x4847EC6: strlen (vg_replace_strmem.c:501)
==135192==    by 0x4B74C85: int_x509_param_set_hosts (x509_vpm.c:46)
==135192==    by 0x12CD5B: openssl::x509::verify::X509VerifyParamRef::set_host (verify.rs:123)
==135192==    by 0x12F25E: openssl::ssl::connector::setup_verify_hostname (connector.rs:400)
==135192==    by 0x12F043: openssl::ssl::connector::ConnectConfiguration::into_ssl (connector.rs:188)
==135192==    by 0x11F610: openssl::ssl::connector::ConnectConfiguration::connect (connector.rs:201)
==135192==    by 0x11A15A: native_tls::imp::TlsConnector::connect (openssl.rs:344)
==135192==    by 0x11D90A: native_tls::TlsConnector::connect (lib.rs:514)
==135192==    by 0x11A981: google_segfault::main (google-segfault.rs:15)
==135192==    by 0x11667A: core::ops::function::FnOnce::call_once (function.rs:248)
==135192==    by 0x11E7BD: std::sys_common::backtrace::__rust_begin_short_backtrace (backtrace.rs:122)
==135192==    by 0x11DFE0: std::rt::lang_start::{{closure}} (rt.rs:166)
==135192==  If you believe this happened as a result of a stack
==135192==  overflow in your program's main thread (unlikely but
==135192==  possible), you can try to increase the size of the
==135192==  main thread stack using the --main-stacksize= flag.
==135192==  The main thread stack size used in this run was 10022912.
==135192== 
==135192== HEAP SUMMARY:
==135192==     in use at exit: 1,008,554 bytes in 17,235 blocks
==135192==   total heap usage: 50,605 allocs, 33,370 frees, 4,551,596 bytes allocated
==135192== 
==135192== LEAK SUMMARY:
==135192==    definitely lost: 0 bytes in 0 blocks
==135192==    indirectly lost: 0 bytes in 0 blocks
==135192==      possibly lost: 0 bytes in 0 blocks
==135192==    still reachable: 1,008,554 bytes in 17,235 blocks
==135192==         suppressed: 0 bytes in 0 blocks
==135192== Rerun with --leak-check=full to see details of leaked memory
==135192== 
==135192== For lists of detected and suppressed errors, rerun with: -s
==135192== ERROR SUMMARY: 2 errors from 1 contexts (suppressed: 0 from 0)

The segfault occurs from:

rust-openssl/openssl/src/ssl/connector.rs

Line 400 in 8e16a8b

Err(_) => param.set_host(domain),

rust-openssl/openssl/src/x509/verify.rs

Line 121 in 8e16a8b

pub fn set_host(&mut self, host: &str) -> Result<(), ErrorStack> {

Solutions:

1. Change the connector

   fn setup_verify_hostname(ssl: &mut SslRef, domain: &str) -> Result<(), ErrorStack> {
       use crate::x509::verify::X509CheckFlags;
   
       let param = ssl.param_mut();
       param.set_hostflags(X509CheckFlags::NO_PARTIAL_WILDCARDS);
       match (domain.parse(), domain.len()) {
           (Ok(ip), _) => param.set_ip(ip),
           (Err(_), 0) => param.set_host(""),
           (Err(_), _) => param.set_host(domain),
       }
   }

2. Change the set_host function to use CString by either changing function signature or converting to an error and handling it:

   /// Set the expected DNS hostname.
   #[corresponds(X509_VERIFY_PARAM_set1_host)]
   pub fn set_host(&mut self, host: &CStr) -> Result<(), ErrorStack> {
       ...
   }

   or

   /// Set the expected DNS hostname.
   #[corresponds(X509_VERIFY_PARAM_set1_host)]
   pub fn set_host(&mut self, host: &str) -> Result<(), ErrorStack> {
       let host = CString::new(host)?;
       unsafe {
           cvt(ffi::X509_VERIFY_PARAM_set1_host(
               self.as_ptr(),
               host.as_ptr() as *const _,
               host.as_bytes().len(),
           ))
           .map(|_| ())
       }
   }