The rat is almost fully functional (for certain values of fully functional), and communication between rats and the server is operational. It is not quite ready, but a new update with a new git commit should hopefully be coming this or next week. While I think everything has proceeded surprisingly smoothly, I have spent many hours debugging stupid mistakes, and this post is about yet another mistake.

The mistake I am going to write about is in the code responsible for connecting back to the server over HTTP. It uses a handle from a call to InternetOpenA to create a HTTP request to a URL, allowing to optionally specify custom headers to be included in the request. The function worked splendidly for performing simple HTTP requests, but the problem showed up when I had to specify custom HTTP headers. A HTTP call was made, but it did not include the specified HTTP headers. Let’s take a look at the code.

 71
 72
 73
 74
 75
 76
 77
 78
 79
 80
 81
 82
 83
 84
 85
 86
 87
 88
 89
 90
 91
 92
 93
 94
 95
 96
 97
 98
 99
100
101
102
103
104
105
106
pub fn create_url_handle(
    &self,
    url: CString,
    headers: Option<CString>,
    flags: InternetUrlFlags,
) -> error::Result<InternetUrlHandle> {
    let handle: *mut c_void;
    let headers_ptr = match headers {
        Some(header_string) => header_string.as_ptr() as *const c_void,
        None => ptr::null::<c_void>(),
    };

    unsafe {
        handle = self.fn_table.call_fn(
            "InternetOpenUrlA".to_string(),
            &[
                arg(&self.handle),
                arg(&url.as_ptr()),
                arg(&headers_ptr),
                arg(&-1i32),
                arg(&flags.bits()),
                arg(&ptr::null::<c_void>()),
            ],
        )?;

        if handle.is_null() {
            return Err(error::Error::WinApiError(GetLastError()));
        }
    }

    Ok(InternetUrlHandle {
        fn_table: self.fn_table,
        handle: handle,
        _internet_handle: self,
    })
}

InternetOpenA expects C-style strings, so CString is used to make sure that a null-byte is added to the end of the string. If Something is provided in the headers parameter, the pointer to the contained CString is sent to InternetOpenUrlA. as_ptr() is used to get the pointer, and if I had actually spent some time reading the manual I should have realized that the code above would case problems. The manual contains the following warning:

The returned pointer is read-only; writing to it (including passing it to C code that writes to it) causes undefined behavior.

It is your responsibility to make sure that the underlying memory is not freed too early. For example, the following code will cause undefined behavior when ptr is used inside the unsafe block:

My limited understanding of what happened is that when a CString is passed in headers, the CString is moved onto the stack on line 79, and then the pointer to the CString (on the stack) is returned. However, after the match statement, the CString goes out of scope and rust is free to use the stack space it occupied for other things. Once I realized what the error was, I simply changed the headers type to Option<&CString>, which may not be a perfect solution, but in this case was good enough and a very simple fix. Since the CString located somewhere outside of create_url_handle’s stack, this approach does not face the same problems caused by the CString going out of scope during the function.

Debugging this was not easy, however. I first started in lldb, but did not manage to find out anything about what happened after libffi handed of execution to InternetOpenUrlA. Using windbg I was able to look at the value passed in r8 when InternetOpenUrlA was called (r8 is the register used to pass the third parameter when using the standard x64 calling convention), and looked at the memory pointed to by that register. The memory did not contain a string, so instead of trusting myself and the debugger, I figured that I surely had done something wrong while debugging.

I resorted to good old fashioned MessageBoxA-style debugging, passing it headers_ptr to display what really was stored at that location. As I should have expected, I was greeted with a message box containing garbage. It was first at this point that I managed to accept that the pointer did indeed not point to a string. Luckily, when I first accepted this, it did not take long to come to figure out what caused the problem and come to the conclusion I did a few paragraphs ago.