From 80cae83e9a6c853ce3a6a2f2c23eb2f549c70be7 Mon Sep 17 00:00:00 2001 From: Alex Murray Date: Tue, 30 Jun 2015 00:59:56 +0200 Subject: Cleanup hddtemp plugin socket reading code to avoid possible memory corruption Make sure we don't write past the bounds of the output buffer - thanks to JeanFI for noticing this issue. --- plugins/hddtemp/hddtemp-plugin.c | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) (limited to 'plugins') diff --git a/plugins/hddtemp/hddtemp-plugin.c b/plugins/hddtemp/hddtemp-plugin.c index 6822338..af37bb3 100644 --- a/plugins/hddtemp/hddtemp-plugin.c +++ b/plugins/hddtemp/hddtemp-plugin.c @@ -36,6 +36,10 @@ #include #endif +#ifdef HAVE_STRING_H +#include +#endif + #include #include #include "hddtemp-plugin.h" @@ -54,11 +58,13 @@ enum { }; +static gchar buffer[HDDTEMP_OUTPUT_BUFFER_LENGTH]; + static const gchar *hddtemp_plugin_query_hddtemp_daemon(GError **error) { int sockfd; ssize_t n = 1; - gboolean first_run = FALSE; - gint output_length = 0; + guint output_length = 0; + static gboolean first_run = TRUE; gchar *pc; struct sockaddr_in address; @@ -66,11 +72,9 @@ static const gchar *hddtemp_plugin_query_hddtemp_daemon(GError **error) { static GTimeVal previous_query_time; GTimeVal current_query_time; - if (NULL == buffer) { - // initialise buffer and current time - buffer = g_new0(char, HDDTEMP_OUTPUT_BUFFER_LENGTH); + if (first_run) { + // initialise previous time g_get_current_time(&previous_query_time); - first_run = TRUE; } g_get_current_time(¤t_query_time); @@ -85,29 +89,27 @@ static const gchar *hddtemp_plugin_query_hddtemp_daemon(GError **error) { g_set_error(error, SENSORS_APPLET_PLUGIN_ERROR, HDDTEMP_SOCKET_OPEN_ERROR, "Error opening socket for hddtemp"); return NULL; } - + address.sin_family = AF_INET; address.sin_addr.s_addr = inet_addr(HDDTEMP_SERVER_IP_ADDRESS); address.sin_port = htons(HDDTEMP_PORT_NUMBER); - if (connect(sockfd, (struct sockaddr *)&address, (socklen_t)sizeof(address)) == -1) { + if (connect(sockfd, (struct sockaddr *)&address, + (socklen_t)sizeof(address)) == -1) { g_set_error(error, SENSORS_APPLET_PLUGIN_ERROR, HDDTEMP_SOCKET_CONNECT_ERROR, "Error connecting to hddtemp daemon on port %i on %s", htons(HDDTEMP_PORT_NUMBER), HDDTEMP_SERVER_IP_ADDRESS); return NULL; } - + memset(buffer, 0, sizeof(buffer)); pc = buffer; - while ((n = read(sockfd, pc, HDDTEMP_OUTPUT_BUFFER_LENGTH - output_length)) > 0) { + while ((n = read(sockfd, pc, + sizeof(buffer) - output_length)) > 0) { output_length += n; - pc = &pc[n]; + pc += n; } - /* terminate with pipe if not already terminated */ - if (buffer[n - 1] != '|') { - buffer[n++] = '|'; - } - /* always null terminate the end of the buffer - * regardless of how much stuff is in it already */ - buffer[output_length] = '\0'; + /* always null terminate the end of the buffer */ + buffer[MIN(output_length, sizeof(buffer) - 1)] = '\0'; close(sockfd); + first_run = FALSE; } return buffer; @@ -161,7 +163,6 @@ static void hddtemp_plugin_get_sensors(GList **sensors) { pv += 5; } g_strfreev(output_vector); - } /* to be called to setup for hddtemp sensors */ -- cgit v1.2.1