8229378: jdwp library loader in linker_md.c quietly truncates on buffer overflow
authorafarley
Wed, 11 Sep 2019 23:10:14 +0000
changeset 58092 b00b4706ec0f
parent 58091 c0cc906cb29c
child 58093 50e1d346a126
8229378: jdwp library loader in linker_md.c quietly truncates on buffer overflow Summary: Check buffer overflow when the jdwp agent full dll name is built Reviewed-by: cjplummer, sspitsyn
src/jdk.jdwp.agent/unix/native/libjdwp/linker_md.c
src/jdk.jdwp.agent/windows/native/libjdwp/linker_md.c
--- a/src/jdk.jdwp.agent/unix/native/libjdwp/linker_md.c	Wed Sep 11 16:06:09 2019 -0700
+++ b/src/jdk.jdwp.agent/unix/native/libjdwp/linker_md.c	Wed Sep 11 23:10:14 2019 +0000
@@ -34,6 +34,7 @@
 #include <stdlib.h>
 #include <string.h>
 
+#include "util.h"
 #include "path_md.h"
 
 #ifdef __APPLE__
@@ -45,8 +46,10 @@
 static void dll_build_name(char* buffer, size_t buflen,
                            const char* paths, const char* fname) {
     char *path, *paths_copy, *next_token;
+    *buffer = '\0';
 
-    paths_copy = strdup(paths);
+    paths_copy = jvmtiAllocate((int)strlen(paths) + 1);
+    strcpy(paths_copy, paths);
     if (paths_copy == NULL) {
         return;
     }
@@ -55,15 +58,18 @@
     path = strtok_r(paths_copy, PATH_SEPARATOR, &next_token);
 
     while (path != NULL) {
-        snprintf(buffer, buflen, "%s/lib%s." LIB_SUFFIX, path, fname);
-        if (access(buffer, F_OK) == 0) {
+        size_t result_len = (size_t)snprintf(buffer, buflen, "%s/lib%s." LIB_SUFFIX, path, fname);
+        if (result_len >= buflen) {
+            EXIT_ERROR(JVMTI_ERROR_INVALID_LOCATION, "One or more of the library paths supplied to jdwp, "
+                                                     "likely by sun.boot.library.path, is too long.");
+        } else if (access(buffer, F_OK) == 0) {
             break;
         }
         *buffer = '\0';
         path = strtok_r(NULL, PATH_SEPARATOR, &next_token);
     }
 
-    free(paths_copy);
+    jvmtiDeallocate(paths_copy);
 }
 
 /*
@@ -89,13 +95,11 @@
 {
     const int pnamelen = pname ? strlen(pname) : 0;
 
-    *holder = '\0';
-    // Quietly truncate on buffer overflow.  Should be an error.
-    if (pnamelen + (int)strlen(fname) + 10 > holderlen) {
-        return;
-    }
-
     if (pnamelen == 0) {
+        if (pnamelen + (int)strlen(fname) + 10 > holderlen) {
+            EXIT_ERROR(JVMTI_ERROR_INVALID_LOCATION, "One or more of the library paths supplied to jdwp, "
+                                                     "likely by sun.boot.library.path, is too long.");
+        }
         (void)snprintf(holder, holderlen, "lib%s." LIB_SUFFIX, fname);
     } else {
       dll_build_name(holder, holderlen, pname, fname);
--- a/src/jdk.jdwp.agent/windows/native/libjdwp/linker_md.c	Wed Sep 11 16:06:09 2019 -0700
+++ b/src/jdk.jdwp.agent/windows/native/libjdwp/linker_md.c	Wed Sep 11 23:10:14 2019 +0000
@@ -37,13 +37,16 @@
 
 #include "sys.h"
 
+#include "util.h"
 #include "path_md.h"
 
 static void dll_build_name(char* buffer, size_t buflen,
                            const char* paths, const char* fname) {
     char *path, *paths_copy, *next_token;
+    *buffer = '\0';
 
-    paths_copy = strdup(paths);
+    paths_copy = jvmtiAllocate((int)strlen(paths) + 1);
+    strcpy(paths_copy, paths);
     if (paths_copy == NULL) {
         return;
     }
@@ -52,15 +55,18 @@
     path = strtok_s(paths_copy, PATH_SEPARATOR, &next_token);
 
     while (path != NULL) {
-        _snprintf(buffer, buflen, "%s\\%s.dll", path, fname);
-        if (_access(buffer, 0) == 0) {
+        size_t result_len = (size_t)_snprintf(buffer, buflen, "%s\\%s.dll", path, fname);
+        if (result_len >= buflen) {
+            EXIT_ERROR(JVMTI_ERROR_INVALID_LOCATION, "One or more of the library paths supplied to jdwp, "
+                                                     "likely by sun.boot.library.path, is too long.");
+        } else if (_access(buffer, 0) == 0) {
             break;
         }
         *buffer = '\0';
         path = strtok_s(NULL, PATH_SEPARATOR, &next_token);
     }
 
-    free(paths_copy);
+    jvmtiDeallocate(paths_copy);
 }
 
 /*
@@ -107,13 +113,11 @@
 {
     const int pnamelen = pname ? (int)strlen(pname) : 0;
 
-    *holder = '\0';
-    /* Quietly truncates on buffer overflow. Should be an error. */
-    if (pnamelen + (int)strlen(fname) + 10 > holderlen) {
-        return;
-    }
-
     if (pnamelen == 0) {
+        if (pnamelen + (int)strlen(fname) + 10 > holderlen) {
+                EXIT_ERROR(JVMTI_ERROR_INVALID_LOCATION, "One or more of the library paths supplied to jdwp, "
+                                                         "likely by sun.boot.library.path, is too long.");
+        }
         sprintf(holder, "%s.dll", fname);
     } else {
       dll_build_name(holder, holderlen, pname, fname);