Fix for CVE-2015-3222 which allows for root escalation via syscheck

Affected versions: 2.7 - 2.8.1

Beginning is OSSEC 2.7 (d88cf1c) a feature was added to syscheck, which
is the daemon that monitors file changes on a system, called
"report_changes". This feature is only available on *NIX systems. It's
purpose is to help determine what about a file has changed. The logic to
do accomplish this is as follows which can be found in
src/syscheck/seechanges.c:

252 /* Run diff */
253 date_of_change = File_DateofChange(old_location);
254 snprintf(diff_cmd, 2048, "diff \"%s\" \"%s\" > \"%s/local/%s/diff.%d\" "
255     "2>/dev/null",
256     tmp_location, old_location,
257     DIFF_DIR_PATH, filename + 1, (int)date_of_change);
258 if (system(diff_cmd) != 256) {
259     merror("%s: ERROR: Unable to run diff for %s",
260            ARGV0,  filename);
261     return (NULL);
262 }

Above, on line 258, the system() call is used to shell out to the
system's "diff" command. The raw filename is passed in as an argument
which presents an attacker with the possibility to run arbitrary code.
Since the syscheck daemon runs as the root user so it can inspect any
file on the system for changes, any code run using this vulnerability
will also be run as the root user.

An example attack might be creating a file called "foo-$(touch bar)"
which should create another file "bar".

Again, this vulnerability exists only on *NIX systems and is contingent
on the following criteria:

1. A vulnerable version is in use.
2. The OSSEC agent is configured to use syscheck to monitor the file
system for changes.
3. The list of directories monitored by syscheck includes those writable
by underprivileged users.
4. The "report_changes" option is enabled for any of those directories.

The fix for this is to create temporary trusted file names that symlink
back to the original files before calling system() and running the
system's "diff" command.

src/InstallAgent.sh

@@ -15,7 +15,7 @@ UNAME=`uname`;
 DIR=`grep DIR ${LOCATION} | cut -f2 -d\"`
 GROUP="ossec"
 USER="ossec"
-subdirs="logs bin queue queue/ossec queue/alerts queue/syscheck queue/rids queue/diff var var/run etc etc/shared active-response active-response/bin agentless .ssh"
+subdirs="logs bin queue queue/ossec queue/alerts queue/syscheck queue/rids queue/diff tmp var var/run etc etc/shared active-response active-response/bin agentless .ssh"
 
 
 # ${DIR} must be set 

src/error_messages/error_messages.h

@@ -50,6 +50,8 @@
 #define RENAME_ERROR  "%s(1124): ERROR: Unable to rename file: '%s'."
 #define INT_ERROR     "%s(1125): ERROR: Internal error (undefined)."
 #define OPEN_ERROR    "%s(1126): ERROR: Unable to open file '%s' reason '%s'"
+#define LINK_ERROR    "%s(1134): ERROR: Unable to link from '%s' to '%s' due to [(%d)-(%s)]."
+
 
 
 /* COMMON ERRORS */

src/headers/defs.h

@@ -278,6 +278,8 @@ published by the Free Software Foundation. For more details, go to \n\
 
 #define WAIT_FILE_PATH  DEFAULTDIR WAIT_FILE
 
+#define TMP_DIR "tmp"
+
 
 /* Default ports */
 #ifndef DEFAULT_SECURE

src/syscheckd/seechanges.c

@@ -219,13 +219,22 @@ int seechanges_createpath(char *filename)
 /* Checks if the file has changed */
 char *seechanges_addfile(char *filename)
 {
-    int date_of_change;
+    time_t old_date_of_change;
+    time_t new_date_of_change;
+
     char old_location[OS_MAXSTR +1];
     char tmp_location[OS_MAXSTR +1];
+    char diff_location[OS_MAXSTR + 1];
+    char old_tmp[OS_MAXSTR + 1];
+    char new_tmp[OS_MAXSTR + 1];
+    char diff_tmp[OS_MAXSTR + 1];
+
     char diff_cmd[OS_MAXSTR +1];
 
     os_md5 md5sum_old;
     os_md5 md5sum_new;
+    int status = -1;
+
 
     old_location[OS_MAXSTR] = '\0';
     tmp_location[OS_MAXSTR] = '\0';
@@ -234,8 +243,14 @@ char *seechanges_addfile(char *filename)
     md5sum_old[0] = '\0';
 
 
-    snprintf(old_location, OS_MAXSTR, "%s/local/%s/%s", DIFF_DIR_PATH, filename +1,
-             DIFF_LAST_FILE);
+    snprintf(
+        old_location,
+        sizeof(old_location),
+        "%s/local/%s/%s",
+        DIFF_DIR_PATH,
+        filename + 1,
+        DIFF_LAST_FILE
+    );
 
 
     /* If the file is not there, rename new location to last location. */
@@ -267,9 +282,18 @@ char *seechanges_addfile(char *filename)
 
 
     /* Saving the old file at timestamp and renaming new to last. */
-    date_of_change = File_DateofChange(old_location);
-    snprintf(tmp_location, OS_MAXSTR, "%s/local/%s/state.%d", DIFF_DIR_PATH, filename +1,
-             date_of_change);
+    old_date_of_change = File_DateofChange(old_location);
+
+    snprintf(
+        tmp_location,
+        sizeof(tmp_location),
+        "%s/local/%s/state.%d",
+        DIFF_DIR_PATH,
+        filename + 1,
+       (int)old_date_of_change
+    );
+
+
     rename(old_location, tmp_location);
     if(seechanges_dupfile(filename, old_location) != 1)
     {
@@ -278,25 +302,99 @@ char *seechanges_addfile(char *filename)
     }
 
 
+    new_date_of_change = File_DateofChange(old_location);
+
+    /* Create file names */
+    snprintf(
+        old_tmp,
+        sizeof(old_tmp),
+        "%s/syscheck-changes-%s-%d",
+        TMP_DIR,
+        md5sum_old,
+        (int)old_date_of_change
+    );
+
+    snprintf(
+        new_tmp,
+        sizeof(new_tmp),
+        "%s/syscheck-changes-%s-%d",
+        TMP_DIR,
+        md5sum_new,
+        (int)new_date_of_change
+    );
+
+    snprintf(
+        diff_tmp,
+        sizeof(diff_tmp),
+        "%s/syscheck-changes-%s-%d-%s-%d",
+        TMP_DIR,
+        md5sum_old,
+        (int)old_date_of_change,
+        md5sum_new,
+        (int)new_date_of_change
+    );
+    /* Create diff location */
+    snprintf(
+        diff_location,
+        sizeof(diff_location),
+        "%s/local/%s/diff.%d",
+        DIFF_DIR_PATH,
+        filename + 1,
+        (int)new_date_of_change
+    );
+
+    /* Create symlinks */
+    if (symlink(old_location, old_tmp) == -1) {
+        merror(LINK_ERROR, ARGV0, old_location, old_tmp, errno, strerror(errno));
+        goto cleanup;
+    }
+
+    if (symlink(tmp_location, new_tmp) == -1) {
+        merror(LINK_ERROR, ARGV0, tmp_location, new_tmp, errno, strerror(errno));
+        goto cleanup;
+    }
+
+    if (symlink(diff_location, diff_tmp) == -1) {
+        merror(LINK_ERROR, ARGV0, diff_location, diff_tmp, errno, strerror(errno));
+        goto cleanup;
+    }
+
+
+
     /* Run diff. */
-    date_of_change = File_DateofChange(old_location);
-    snprintf(diff_cmd, 2048, "diff \"%s\" \"%s\" > \"%s/local/%s/diff.%d\" "
-             "2>/dev/null",
-             tmp_location, old_location,
-             DIFF_DIR_PATH, filename +1, date_of_change);
-    if(system(diff_cmd) != 256)
-    {
-        merror("%s: ERROR: Unable to run diff for %s",
-               ARGV0,  filename);
-        return(NULL);
+    snprintf(
+        diff_cmd,
+        2048,
+        "diff \"%s\" \"%s\" > \"%s\" 2> /dev/null",
+        new_tmp,
+        old_tmp,
+        diff_tmp
+    );
+
+
+
+    if(system(diff_cmd) != 256) {
+        merror("%s: ERROR: Unable to run diff for %s", ARGV0, filename);
+        goto cleanup;
+
     }
 
+    /* Success */
+    status = 0;
 
-    /* Generate alert. */
-    return(gen_diff_alert(filename, date_of_change));
+cleanup:
+    unlink(old_tmp);
+    unlink(new_tmp);
+    unlink(diff_tmp);
+
+    if (status == -1)
+        return (NULL);
 
 
-    return(NULL);
+
+    /* Generate alert. */
+    return (gen_diff_alert(filename, new_date_of_change));
+
 }