Pengpeng Hou created THRIFT-5931:
------------------------------------
Summary: thrift_ssl_socket_get_ssl_error() can underflow its
remaining-buffer counter and write past the stack buffer
Key: THRIFT-5931
URL: https://issues.apache.org/jira/browse/THRIFT-5931
Project: Thrift
Issue Type: Bug
Reporter: Pengpeng Hou
Hi ,
I rechecked current upstream head and this SSL error formatting path still
looks unsafe.
In `lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c`,
`thrift_ssl_socket_get_ssl_error()` builds a formatted message in a fixed `char
buffer[1024]` and tracks the remaining space with an `int buffer_size = 1024`.
The code subtracts the return value of `snprintf()` from `buffer_size` and then
reuses that counter and the derived pointer for later appends:
buffer_size -= snprintf(buffer, buffer_size, "SSL %s: ", error_msg);
...
buffer_size -= snprintf(buffer + (1024 - buffer_size), buffer_size, "\n\t");
buffer_size -= snprintf(buffer + (1024 - buffer_size), buffer_size,
"%lX(%s) -> %s", ...);
The problem is that `snprintf()` returns the length it tried to produce, not a
clamped remaining size. Once the formatted text becomes longer than the
remaining space, `buffer_size` can go negative. After that, the next
subtraction and pointer calculation are no longer safe: the code can end up
passing a negative value as the size argument and computing a write pointer
beyond the end of the 1024-byte stack buffer.
I rechecked current head and did not find any guard that stops the append loop
before the remaining-space counter underflows.
*How to reproduce*
1. Build current Apache Thrift with the C GLib SSL transport.
2. Trigger a TLS failure path that produces enough OpenSSL error strings to
keep appending to the local 1024-byte buffer.
3. Once the formatted text exceeds the remaining space, `buffer_size`
underflows and later appends reuse the bad counter.
4. With sanitizers enabled, this shows up as an out-of-bounds write or invalid
pointer arithmetic in `thrift_ssl_socket_get_ssl_error()`.
In practice, any long enough error stack or long enough formatted error content
can drive the same path.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
