[
https://issues.apache.org/jira/browse/THRIFT-5931?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18067326#comment-18067326
]
Jens Geyer commented on THRIFT-5931:
------------------------------------
Hi,
we accept pull requests.
[Apache Thrift - How To
Contribute|https://thrift.apache.org/docs/HowToContribute]
> 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
> Priority: Major
>
> 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()`.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
