RDKB-65595 JST (Generic) Security Fuzzing Report Fixes

source/jst_functions.c

@@ -1,5 +1,5 @@
 /*
  If not stated otherwise in this file or this component's Licenses.txt file the
  following copyright and licenses apply:
 
  Copyright 2018 RDK Management
@@ -21,6 +21,8 @@
 #include <unistd.h>
 #include <errno.h>
 #include <sys/stat.h>
+#include <sys/wait.h>
+#include <wordexp.h>
 #include "jst_internal.h"
 #include "jst.h"
 
@@ -153,6 +155,12 @@
   ssize_t nread;
   duk_idx_t idx;
   int index = 0;
+  wordexp_t args;
+  int wr;
+  int pipes[2] = { -1, -1 };
+  pid_t pid;
+  FILE* pipe_stream = NULL;
+  int status = 0;
 
   idx = duk_push_array(ctx);
 
@@ -161,23 +169,96 @@
 
   CosaPhpExtLog("exec command=%s\n", command);
 
-  FILE* pipe = popen(command, "r");
-  if (!pipe)
+  wr = wordexp(command, &args, WRDE_NOCMD);
+  if (wr != 0)
   {
-    CosaPhpExtLog("exec failed to open pipe\n");
-    duk_pop(ctx);
-    return 1;    
+    CosaPhpExtLog("exec failed to parse command, wordexp status=%d\n", wr);
+    return 1;
+  }
+  if (args.we_wordc == 0)
+  {
+    CosaPhpExtLog("exec empty command after expansion\n");
+    wordfree(&args);
+    return 1;
+  }
+
+  if (pipe(pipes) != 0)
+  {
+    CosaPhpExtLog("exec failed to create pipe, error:%s\n", strerror(errno));
+    wordfree(&args);
+    return 1;
+  }
+
+  pid = fork();
+  if (pid < 0)
+  {
+    CosaPhpExtLog("exec fork failed, error:%s\n", strerror(errno));
+    close(pipes[0]);
+    close(pipes[1]);
+    wordfree(&args);
+    return 1;
+  }
+
+  if (pid == 0)
+  {
+    close(pipes[0]);
+    if (dup2(pipes[1], STDOUT_FILENO) < 0)
+    {
+      _exit(127);
+    }
+    close(pipes[1]);
+    execvp(args.we_wordv[0], args.we_wordv);
+    _exit(127);
   }
 
-  while((nread = getline(&line, &len, pipe)) != -1)
+  close(pipes[1]);
+  pipe_stream = fdopen(pipes[0], "r");
+  if (!pipe_stream)
+  {
+    CosaPhpExtLog("exec failed to open read pipe, error:%s\n", strerror(errno));
+    close(pipes[0]);
+    {
+      pid_t wp;
+      do { wp = waitpid(pid, &status, 0); } while (wp == -1 && errno == EINTR);
+      if (wp == -1)
+        CosaPhpExtLog("exec waitpid failed, error:%s\n", strerror(errno));
+    }
+    wordfree(&args);
+    return 1;
+  }
+
+  while((nread = getline(&line, &len, pipe_stream)) != -1)
   {
     CosaPhpExtLog("exec line: %s\n", line);
     duk_push_string(ctx, line);
     duk_put_prop_index(ctx, idx, index++);
   }
 
   free(line);
-  pclose(pipe);
+  fclose(pipe_stream);
+  {
+    pid_t wp;
+    do { wp = waitpid(pid, &status, 0); } while (wp == -1 && errno == EINTR);
+    if (wp == -1)
+    {
+      CosaPhpExtLog("exec waitpid failed, error:%s\n", strerror(errno));
+    }
+    else if (WIFEXITED(status))
+    {
+      int exit_code = WEXITSTATUS(status);
+      if (exit_code != 0)
+        CosaPhpExtLog("exec command exited with code %d\n", exit_code);
+    }
+    else if (WIFSIGNALED(status))
+    {
+      CosaPhpExtLog("exec command killed by signal %d\n", WTERMSIG(status));
+    }
+    else
+    {
+      CosaPhpExtLog("exec command exited abnormally, raw status=%d\n", status);
+    }
+  }
+  wordfree(&args);
 
   return 1;
 }
@@ -615,7 +696,7 @@
   /* === NOW PROCEED WITH SIGNATURE VERIFICATION === */
 
   //open certificate file
-  if(memcmp(filepath, "file://", sizeof("file://")-1) != 0)
+  if(strncmp(filepath, "file://", sizeof("file://") - 1) != 0)
   {
     CosaPhpExtLog("openssl_verify_with_cert: file %s doesn't begin with 'file://'\n", filepath);
     free(sig_bytes);

source/jst_internal.c

@@ -1,5 +1,5 @@
 /*
  If not stated otherwise in this file or this component's Licenses.txt file the
  following copyright and licenses apply:
 
  Copyright 2018 RDK Management
@@ -19,6 +19,7 @@
 #include "jst_internal.h"
 #include <stdio.h>
 #include <errno.h>
+#include <stdint.h>
 #include <sys/types.h>
 #include <sys/stat.h>
 
@@ -179,10 +180,41 @@
     return 0;
   }
 
-  fseek(pf, 0, SEEK_END); 
-  size = ftell(pf);
+  if(fseek(pf, 0, SEEK_END) != 0)
+  {
+    fclose(pf);
+    fprintf(stderr, "Error: fseek failed %s\n", filename);
+    return 0;
+  }
+  {
+    long ftell_result = ftell(pf);
+    if(ftell_result < 0)
+    {
+      fclose(pf);
+      fprintf(stderr, "Error: ftell failed %s (not a regular file?)\n", filename);
+      return 0;
+    }
+
+#if LONG_MAX > SIZE_MAX
+    if((unsigned long)ftell_result > (unsigned long)SIZE_MAX)
+    {
+      fclose(pf);
+      fprintf(stderr, "Error: file too large to represent safely %s\n", filename);
+      return 0;
+    }
+#endif
+
+    size = (size_t)ftell_result;
+  }
   rewind(pf);
 
+  if(size > ((size_t)-1) - 1)
+  {
+    fclose(pf);
+    fprintf(stderr, "Error: file size overflow %s\n", filename);
+    return 0;
+  }
+
   buf = (char*)calloc(size+1, 1);
   if(!buf)
   {

source/jst_session.c

@@ -112,34 +112,50 @@ static duk_ret_t session_start(duk_context *ctx)
     if(sesid)
     {
       sesid += 7;
-      int len = strlen(sesid);
-      if(len >= SESSION_ID_LENGTH)
+      size_t sesid_token_len = strcspn(sesid, ";");
+      if(sesid_token_len == SESSION_ID_LENGTH)
       {
-           int idx = SESSION_PREFIX_LEN;
-           int isvalid = 1;
-           /* Validate session ID*/
-           while ( idx < SESSION_ID_LENGTH) {
-              if (!isalnum(sesid[idx])) {
-                      CosaPhpExtLog("Invalid SessionID\n");
-                      isvalid = 0;
-                      break;
-              }
-              idx++;
-           }
-           if(isvalid)
-           {
-             sesid = strtok(sesid, ";");
-             const char filename[SESSION_FILE_MAX_PATH];
-             snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid);
-             CosaPhpExtLog("%s: Checking for Session file %s\n", __PRETTY_FUNCTION__, filename);
-             if (access(filename, F_OK) == 0)
-             {
-               CosaPhpExtLog("%s: Session file %s exists\n", __PRETTY_FUNCTION__, filename);
-               strncpy(session_identifier, sesid, SESSION_ID_LENGTH);
-             } else {
-               CosaPhpExtLog("%s: Failed to read Session file %s\n", __PRETTY_FUNCTION__, filename);
-             }
-           }
+        int idx = SESSION_PREFIX_LEN;
+        int isvalid = 1;
+        char sesid_token[SESSION_ID_LENGTH+1];
+
+        memcpy(sesid_token, sesid, SESSION_ID_LENGTH);
+        sesid_token[SESSION_ID_LENGTH] = '\0';
+
+        if(strncmp(sesid_token, SESSION_PREFIX, SESSION_PREFIX_LEN) != 0)
+        {
+          CosaPhpExtLog("Invalid SessionID prefix\n");
+          isvalid = 0;
+        }
+
+        /* Validate random portion of session ID */
+        while (idx < SESSION_ID_LENGTH && isvalid)
+        {
+          if(!isalnum((unsigned char)sesid_token[idx]))
+          {
+            CosaPhpExtLog("Invalid SessionID\n");
+            isvalid = 0;
+            break;
+          }
+          idx++;
+        }
+
+        if(isvalid)
+        {
+          char filename[SESSION_FILE_MAX_PATH];
+          snprintf(filename, SESSION_FILE_MAX_PATH, "%s/%s", SESSION_TMP_DIR, sesid_token);
+          CosaPhpExtLog("%s: Checking for Session file %s\n", __PRETTY_FUNCTION__, filename);
+          if (access(filename, F_OK) == 0)
+          {
+            CosaPhpExtLog("%s: Session file %s exists\n", __PRETTY_FUNCTION__, filename);
+            strncpy(session_identifier, sesid_token, SESSION_ID_LENGTH);
+            session_identifier[SESSION_ID_LENGTH] = '\0';
+          }
+          else
+          {
+            CosaPhpExtLog("%s: Failed to read Session file %s\n", __PRETTY_FUNCTION__, filename);
+          }
+        }
       } else {
            CosaPhpExtLog("Invalid SessionID Entropy\n");
       }
@@ -162,8 +178,15 @@ static duk_ret_t session_create(duk_context *ctx)
   int i = 0, n = 0;
   uint8_t bytes[SESSION_ID_BYTES_LENGTH];
   char* session_id = NULL;
+  char* new_session_identifier = NULL;
 
   session_id = (char*)malloc(SESSION_ID_BYTES_LENGTH+1);
+  if(!session_id)
+  {
+    CosaPhpExtLog("Failed to allocate session_id!\n");
+    RETURN_FALSE;
+  }
+
   n = syscall(SYS_getrandom, bytes, SESSION_ID_BYTES_LENGTH, 0);
   if(n != SESSION_ID_BYTES_LENGTH)
   {
@@ -177,16 +200,25 @@ static duk_ret_t session_create(duk_context *ctx)
     session_id[i] = BYTE_TO_PRINTABLE_HEX_CODE(bytes[i]);
   }
 
-  session_identifier = (char*)malloc(SESSION_ID_LENGTH+1);
-  if(!session_identifier)
+  new_session_identifier = (char*)malloc(SESSION_ID_LENGTH+1);
+  if(!new_session_identifier)
   {
     CosaPhpExtLog("Failed to allocate session_identifier!\n");
+    free(session_id);
     RETURN_FALSE;
   }
-  memset(session_identifier, 0, SESSION_ID_LENGTH+1);
+  memset(new_session_identifier, 0, SESSION_ID_LENGTH+1);
 
   session_id[SESSION_ID_BYTES_LENGTH] = '\0';
-  snprintf(session_identifier, SESSION_ID_LENGTH+1, "%s%s", SESSION_PREFIX, session_id);
+  snprintf(new_session_identifier, SESSION_ID_LENGTH+1, "%s%s", SESSION_PREFIX, session_id);
+
+  free(session_id);
+
+  if(session_identifier)
+  {
+    free(session_identifier);
+  }
+  session_identifier = new_session_identifier;
 
   RETURN_TRUE;
   return 1;