Skip to content

Claude Opus 4.6: Found 5 Bugs in v1.0.0. #94

@globalskymail-ctrl

Description

@globalskymail-ctrl

Hello,

Has anyone run WinTLS thru the latest coding AI's such as Claude/Gemini/ChatGPT/... to determine bugs?

Below is a plan created by Claude Opus 4.6 and it found 5 Bugs.

Are they valid bugs or just AI slop? (I don't have the expertise/time to validate the accuracy.)

It would be interesting to see what other AI's report when analyzing the WinTLS code for bugs.

Thanks!

Bug Fixes in boost-wintls

Context
Code review of the boost-wintls library identified five bugs: a type mismatch in a crypto API call, an unchecked return value that can crash, data loss during TLS renegotiation, a redundant SSPI call in the async shutdown coroutine, and a memory leak in move-assignment.

Bug 1: Wrong type in delete_private_key (certificate.hpp:188)

File: include/wintls/certificate.hpp:188

HCRYPTKEY ptr is declared but CryptAcquireContextA expects HCRYPTPROV* as its first argument. Both types are ULONG_PTR on current Windows so this doesn't crash today, but it's semantically wrong and fragile.

Fix: Change HCRYPTKEY ptr = 0; to HCRYPTPROV ptr = 0;

Bug 2: mbstowcs return value unchecked — crash on invalid hostname (context_certificates.hpp:150)

File: include/wintls/detail/context_certificates.hpp:148-150

whostname.resize(server_hostname.size(), L' ');
whostname.resize(std::mbstowcs(&whostname[0], server_hostname.data(), server_hostname.size()));

If mbstowcs encounters an invalid multibyte sequence, it returns (size_t)-1. Passing that to resize() attempts to allocate ~18 exabytes, crashing with std::bad_alloc or worse.

Fix: Check the return value of mbstowcs. If it returns (size_t)-1, return an appropriate error (e.g., GetLastError() or a suitable HRESULT).

Bug 3: Renegotiation discards extra encrypted data (sspi_decrypt.hpp:98-100)

File: include/wintls/detail/sspi_decrypt.hpp:90-100

if (buffers_[3].BufferType == SECBUFFER_EXTRA) {
    const auto extra_size = buffers_[3].cbBuffer;
    std::memmove(encrypted_data_.data(), buffers_[3].pvBuffer, extra_size);
    buffers_[0].cbBuffer = extra_size;   // saves extra data
} else {
    buffers_[0].cbBuffer = 0;
}

if (last_error_ == SEC_I_RENEGOTIATE) {
    buffers_[0].cbBuffer = 0;            // immediately discards it!
    return state::renegotiate_handshake;
}

When DecryptMessage returns SEC_I_RENEGOTIATE with SECBUFFER_EXTRA data, lines 91-93 correctly save the extra data to the front of encrypted_data_, but line 99 unconditionally zeros cbBuffer, losing that data. Post-renegotiation decryption will miss those bytes.

Fix: Remove the buffers_[0].cbBuffer = 0; on line 99, so that the extra data count from lines 91-93 is preserved through the renegotiation.

Bug 4: shutdown_() called redundantly on coroutine re-entry (async_shutdown.hpp:38)

File: include/wintls/detail/async_shutdown.hpp:38

ec = shutdown_() sits outside the WINTLS_ASIO_CORO_REENTER block. On re-entry after async_write completes, the coroutine re-enters operator() from the top and calls shutdown_() again before jumping to the resume point. This fires ApplyControlToken + InitializeSecurityContext a second time on an already-shutting-down SSPI context. The result is discarded because the coroutine resumes past it, but the redundant SSPI state mutation is wrong.

Fix: Move ec = shutdown_() inside the REENTER block, before the if (!ec) check.

WINTLS_ASIO_CORO_REENTER(*this) {
  ec = shutdown_();    // ← call once, on first entry only
  if (!ec) {
    WINTLS_ASIO_CORO_YIELD net::async_write(next_layer_, shutdown_.buffer(), std::move(self));
    shutdown_.size_written(size_written);
    self.complete({});
    return;
  }
  // error path...
}

Bug 5: Move-assignment leaks SSPI buffer (sspi_context_buffer.hpp:30-34)

File: include/wintls/detail/sspi_context_buffer.hpp:30-34

sspi_context_buffer& operator=(sspi_context_buffer&& other) {
    buffer_ = other.buffer_;       // old buffer_ leaked — FreeContextBuffer never called
    other.buffer_ = net::const_buffer{};
    return *this;
}

If this already owns an SSPI-allocated buffer, overwriting buffer_ leaks it.

Fix: Free the old buffer and add self-assignment protection:

sspi_context_buffer& operator=(sspi_context_buffer&& other) {
    if (this != &other) {
        detail::sspi_functions::FreeContextBuffer(const_cast<void*>(buffer_.data()));
        buffer_ = other.buffer_;
        other.buffer_ = net::const_buffer{};
    }
    return *this;
}

Verification
Build the project and run the existing test suite to confirm no regressions
The type fix (Bug 1) and mbstowcs fix (Bug 2) are straightforward correctness fixes
Bug 3 affects a rare code path (renegotiation with extra data); verify with existing echo tests that normal TLS communication still works
Bug 4 and Bug 5 are structural fixes; verify with the full test suitePlan updated with all 5 bugs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions