Skip to content

Commit 0de53c4

Browse files
neosys007Jens-G
authored andcommitted
THRIFT-5931: c_glib: avoid fixed-size buffers in thrift_ssl_socket_get_ssl_error()
thrift_ssl_socket_get_ssl_error() still builds SSL error messages in a fixed stack buffer while tracking remaining space with a signed counter that is updated by subtracting snprintf() return values. If the formatted error text is long enough, that counter can underflow and the later writes can walk past the intended buffer boundaries. Build the error message with GString instead so the helper no longer depends on hand-rolled remaining-space arithmetic. Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
1 parent ee26195 commit 0de53c4

File tree

1 file changed

+14
-15
lines changed

1 file changed

+14
-15
lines changed

lib/c_glib/src/thrift/c_glib/transport/thrift_ssl_socket.c

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -164,42 +164,42 @@ static
164164
void thrift_ssl_socket_get_ssl_error(ThriftSSLSocket *socket, const gchar *error_msg, guint thrift_error_no, int ssl_error, GError **error)
165165
{
166166
unsigned long error_code;
167-
char buffer[1024];
168-
int buffer_size=1024;
167+
GString *message = g_string_new ("");
169168
gboolean first_error = TRUE;
170169
int ssl_error_type = SSL_get_error(socket->ssl, ssl_error);
171170
if(ssl_error_type>0){
172171
switch(ssl_error_type){
173172
case SSL_ERROR_SSL:
174-
buffer_size-=snprintf(buffer, buffer_size, "SSL %s: ", error_msg);
175-
while ((error_code = ERR_get_error()) != 0 && buffer_size>1) {
173+
g_string_append_printf (message, "SSL %s: ", error_msg);
174+
while ((error_code = ERR_get_error()) != 0) {
176175
const char* reason = ERR_reason_error_string(error_code);
177176
if(reason!=NULL){
178177
if(!first_error) {
179-
buffer_size-=snprintf(buffer+(1024-buffer_size), buffer_size, "\n\t");
180-
first_error=FALSE;
178+
g_string_append (message, "\n\t");
181179
}
182-
buffer_size-=snprintf(buffer+(1024-buffer_size), buffer_size, "%lX(%s) -> %s", error_code, reason, SSL_state_string(socket->ssl));
180+
g_string_append_printf (message, "%lX(%s) -> %s", error_code, reason, SSL_state_string(socket->ssl));
181+
first_error = FALSE;
183182
}
184183
}
185184
break;
186185
case SSL_ERROR_SYSCALL:
187-
buffer_size-=snprintf(buffer, buffer_size, "%s: ", error_msg);
188-
buffer_size-=snprintf(buffer+(1024-buffer_size), buffer_size, "%X -> %s", errno, strerror(errno));
186+
g_string_append_printf (message, "%s: ", error_msg);
187+
g_string_append_printf (message, "%X -> %s", errno, strerror(errno));
189188
break;
190189
case SSL_ERROR_WANT_READ:
191-
buffer_size-=snprintf(buffer, buffer_size, "%s: ", error_msg);
192-
buffer_size-=snprintf(buffer+(1024-buffer_size), buffer_size, "%X -> %s", ssl_error_type, "Error while reading from underlaying layer");
190+
g_string_append_printf (message, "%s: ", error_msg);
191+
g_string_append_printf (message, "%X -> %s", ssl_error_type, "Error while reading from underlaying layer");
193192
break;
194193
case SSL_ERROR_WANT_WRITE:
195-
buffer_size-=snprintf(buffer, buffer_size, "%s: ", error_msg);
196-
buffer_size-=snprintf(buffer+(1024-buffer_size), buffer_size, "%X -> %s", ssl_error_type, "Error while writting to underlaying layer");
194+
g_string_append_printf (message, "%s: ", error_msg);
195+
g_string_append_printf (message, "%X -> %s", ssl_error_type, "Error while writting to underlaying layer");
197196
break;
198197

199198
}
200199
g_set_error (error, THRIFT_TRANSPORT_ERROR,
201-
thrift_error_no, "%s", buffer);
200+
thrift_error_no, "%s", message->str);
202201
}
202+
g_string_free (message, TRUE);
203203
}
204204

205205
/**
@@ -867,4 +867,3 @@ thrift_ssl_socket_context_initialize(ThriftSSLSocketProtocol ssl_protocol, GErro
867867

868868
return context;
869869
}
870-

0 commit comments

Comments
 (0)