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.
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
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
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.
Bug 5: Move-assignment leaks SSPI buffer (sspi_context_buffer.hpp:30-34)
File: include/wintls/detail/sspi_context_buffer.hpp:30-34
If this already owns an SSPI-allocated buffer, overwriting buffer_ leaks it.
Fix: Free the old buffer and add self-assignment protection:
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.