[2.7-beta0-csyslogd] avoid potential string buffer overflow

qqlover · Sep 20, 2012 · 6188e26 · 6188e26
1 parent fc3422f
commit 6188e26
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 47 deletions.
diff --git a/LICENSE b/LICENSE
@@ -1,5 +1,5 @@
 
- Copyright (C) 2011 Trend Micro Inc. All rights reserved.
+ Copyright (C) 2012 Trend Micro Inc. All rights reserved.
 
  OSSEC HIDS is a free software; you can redistribute it and/or modify
  it under the terms of the GNU General Public License (version 2) as

diff --git a/etc/templates/en/messages/0x102-installhelp.txt b/etc/templates/en/messages/0x102-installhelp.txt
@@ -16,7 +16,7 @@
       the agents or external syslog devices.
 
     - If you choose 'hybrid', you get the 'local' installation 
-      plus the 'agent' installatoin. 
+      plus the 'agent' installation. 
 
   - Choose 'server' if you are setting up a log/analysis server.
 

diff --git a/src/os_csyslogd/alert.c b/src/os_csyslogd/alert.c
@@ -25,7 +25,7 @@
 int OS_Alert_SendSyslog(alert_data *al_data, SyslogConfig *syslog_config)
 {
     char *tstamp;
-    char syslog_msg[OS_SIZE_2048 +1];
+    char syslog_msg[OS_SIZE_2048];
 
     /* These will be Malloc'd, so no need to predeclare size, just remember to free! */
     char *json_safe_comment;
@@ -40,7 +40,7 @@ int OS_Alert_SendSyslog(alert_data *al_data, SyslogConfig *syslog_config)
 
 
     /* Clearing the memory before insert */
-    memset(syslog_msg, '\0', OS_SIZE_2048 +1);
+    memset(syslog_msg, '\0', OS_SIZE_2048);
 
 
     /* Looking if location is set */
@@ -140,7 +140,7 @@ int OS_Alert_SendSyslog(alert_data *al_data, SyslogConfig *syslog_config)
         field_add_string(syslog_msg, OS_SIZE_2048, " Current MD5: %s;", al_data->new_md5 );
         field_add_string(syslog_msg, OS_SIZE_2048, " Previous SHA1: %s;", al_data->old_sha1 );
         field_add_string(syslog_msg, OS_SIZE_2048, " Current SHA1: %s;", al_data->new_sha1 );
-        field_add_long_string(syslog_msg, OS_SIZE_2048, " %s", al_data->log[0] );
+        field_add_string(syslog_msg, OS_SIZE_2048, " %s", al_data->log[0] );
     }
     else if(syslog_config->format == CEF_CSYSLOG)
     {
@@ -164,12 +164,12 @@ int OS_Alert_SendSyslog(alert_data *al_data, SyslogConfig *syslog_config)
 #endif
         field_add_string(syslog_msg, OS_SIZE_2048, " suser=%s", al_data->user );
         field_add_string(syslog_msg, OS_SIZE_2048, " dst=%s", al_data->dstip );
-        field_add_long_string(syslog_msg, OS_SIZE_2048, " msg=%s", al_data->log[0] );
+        field_add_string(syslog_msg, OS_SIZE_2048, " msg=%s", al_data->log[0] );
     }
     else if(syslog_config->format == JSON_CSYSLOG)
     {
         /* Build a JSON Object for logging */
-        snprintf(syslog_msg, OS_SIZE_2048 - 20,
+        snprintf(syslog_msg, OS_SIZE_2048,
                 "<%d>%s %s ossec: { \"crit\": %d, \"id\": %d, \"description\": \"%s\", \"component\": \"%s\",",
 
                 /* syslog header */
@@ -180,27 +180,27 @@ int OS_Alert_SendSyslog(alert_data *al_data, SyslogConfig *syslog_config)
                 al_data->location
         );
         /* Event specifics */
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"classification\": \"%s\",", al_data->group );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"classification\": \"%s\",", al_data->group );
 
-        if( field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"src_ip\": \"%s\",", al_data->srcip ) > 0 )
-            field_add_int(syslog_msg, OS_SIZE_2048 - 20, " \"src_port\": %d,", al_data->srcport );
+        if( field_add_string(syslog_msg, OS_SIZE_2048, " \"src_ip\": \"%s\",", al_data->srcip ) > 0 )
+            field_add_int(syslog_msg, OS_SIZE_2048, " \"src_port\": %d,", al_data->srcport );
 
 #ifdef GEOIP
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"src_city\": \"%s\",", al_data->geoipdatasrc );
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"dst_city\": \"%s\",", al_data->geoipdatadst );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"src_city\": \"%s\",", al_data->geoipdatasrc );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"dst_city\": \"%s\",", al_data->geoipdatadst );
 #endif
 
-        if ( field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"dst_ip\": \"%s\",", al_data->dstip ) > 0 )
-            field_add_int(syslog_msg, OS_SIZE_2048 - 20, " \"dst_port\": %d,", al_data->dstport );
+        if ( field_add_string(syslog_msg, OS_SIZE_2048, " \"dst_ip\": \"%s\",", al_data->dstip ) > 0 )
+            field_add_int(syslog_msg, OS_SIZE_2048, " \"dst_port\": %d,", al_data->dstport );
 
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"file\": \"%s\",", al_data->filename );
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"acct\": \"%s\",", al_data->user );
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"md5_old\": \"%s\",", al_data->old_md5 );
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"md5_new\": \"%s\",", al_data->new_md5 );
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"sha1_old\": \"%s\",", al_data->old_sha1 );
-        field_add_string(syslog_msg, OS_SIZE_2048 - 20, " \"sha1_new\": \"%s\",", al_data->new_sha1 );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"file\": \"%s\",", al_data->filename );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"acct\": \"%s\",", al_data->user );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"md5_old\": \"%s\",", al_data->old_md5 );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"md5_new\": \"%s\",", al_data->new_md5 );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"sha1_old\": \"%s\",", al_data->old_sha1 );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"sha1_new\": \"%s\",", al_data->new_sha1 );
 		/* Message */
-        field_add_long_string(syslog_msg, OS_SIZE_2048 - 4, " \"message\": \"%s", json_safe_message );
+        field_add_string(syslog_msg, OS_SIZE_2048, " \"message\": \"%s", json_safe_message );
         field_add_string(syslog_msg, OS_SIZE_2048, "\" }", "" ); //always add closing double quote and brace
     }
     else if(syslog_config->format == SPLUNK_CSYSLOG)

diff --git a/src/os_csyslogd/csyslogd.c b/src/os_csyslogd/csyslogd.c
@@ -14,7 +14,11 @@
  */
 
 
-
+/* strnlen is a GNU extension */
+#ifdef __linux__
+ #define _GNU_SOURCE
+ #include <string.h>
+#endif
 #include "csyslogd.h"
 #include "os_net/os_net.h"
 
@@ -112,9 +116,15 @@ char *strip_double_quotes(char *source) {
 }
 
 /* Format Field for output */
-unsigned int field_add_string(char *dest, unsigned int size, const char *format, const char *value ) {
-    char buffer[255];
-    unsigned int len = 0;
+int field_add_string(char *dest, int size, const char *format, const char *value ) {
+    char buffer[OS_SIZE_2048];
+    int len = 0;
+    int dest_sz = size - strnlen(dest, OS_SIZE_2048);
+
+    if(dest_sz <= 0 ) {
+        // Not enough room in the buffer
+        return -1;
+    }
 
     if(value != NULL &&
             (
@@ -123,35 +133,27 @@ unsigned int field_add_string(char *dest, unsigned int size, const char *format,
                 ((value[0] != 'u') && (value[1] != 'n') && (value[4] != 'k'))
             )
     ) {
-        len = snprintf(buffer, 255, format, value);
-        strncat(dest, buffer, OS_SIZE_2048);
-    }
-
-    return len;
-}
-
-/* Add long string */
-unsigned int field_add_long_string(char *dest, unsigned int size, const char *format, const char *value ) {
-    char buffer[OS_SIZE_2048 + 1];
-    unsigned int len = 0;
-    unsigned int dest_sz = strlen(dest);
-
-    if(value != NULL) {
-        len = snprintf(buffer, OS_SIZE_2048 - dest_sz - 2  , format, value);
-        strncat(dest, buffer, size);
+        len = snprintf(buffer, sizeof(buffer) - dest_sz - 1, format, value);
+        strncat(dest, buffer, dest_sz);
     }
 
     return len;
 }
 
 /* Handle integers in the second position */
-unsigned int field_add_int(char *dest, unsigned int size, const char *format, const int value ) {
+int field_add_int(char *dest, int size, const char *format, const int value ) {
     char buffer[255];
-    unsigned int len = 0;
+    int len = 0;
+    int dest_sz = size - strnlen(dest, OS_SIZE_2048);
+
+    if(dest_sz <= 0 ) {
+        // Not enough room in the buffer
+        return -1;
+    }
 
     if( value > 0 ) {
-        len = snprintf(buffer, 255, format, value);
-        strncat(dest, buffer, OS_SIZE_2048);
+        len = snprintf(buffer, sizeof(buffer), format, value);
+        strncat(dest, buffer, dest_sz);
     }
 
     return len;

diff --git a/src/os_csyslogd/csyslogd.h b/src/os_csyslogd/csyslogd.h
@@ -39,8 +39,8 @@ void OS_CSyslogD(SyslogConfig **syslog_config);
 char *strip_double_quotes(char* source);
 
 /* Conditional Field Formatting */
-unsigned int field_add_int(char *dest, unsigned int size, const char *format, const int value );
-unsigned int field_add_string(char *dest, unsigned int size, const char *format, const char *value );
+int field_add_int(char *dest, int size, const char *format, const int value );
+int field_add_string(char *dest, int size, const char *format, const char *value );
 
 
 /** Global vars **/