diff --git a/CHANGES.md b/CHANGES.md index 9e90967fa2..2ee88bd265 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -2,6 +2,7 @@ Next Release (4.0) ================== NOTE: as of JNA 4.0, JNA is now dual-licensed under LGPL and ASL (see LICENSE). + NOTE: JNI native support is typically incompatible between minor versions, and almost always incompatible between major versions. Features @@ -32,6 +33,7 @@ Bug Fixes * Avoid solaris/x86 JVM bug w/library open flags - [@twall](https://github.com/twall). * Fixed NPE returning wide string from a direct-mapped function - [@twall](https://github.com/twall). * [#237](https://github.com/twall/jna/issues/237): Fix LastErrorException/getLastError on AIX - [@skissane](https://github.com/skissane). +* [#228](https://github.com/twall/jna/issues/228): Fix win32/win64 crashes due to LastErrorException buffer overruns (`snprintf` on windows is broken) - [@davidhoyt](https://github.com/davidhoyt). Release 3.5.2 ============= diff --git a/lib/native/win32-x86-64.jar b/lib/native/win32-x86-64.jar index 93df1ad0e9..595aa92c97 100755 Binary files a/lib/native/win32-x86-64.jar and b/lib/native/win32-x86-64.jar differ diff --git a/native/Makefile b/native/Makefile index 02df8f75cd..48d3dc9f11 100644 --- a/native/Makefile +++ b/native/Makefile @@ -167,7 +167,7 @@ ifeq ($(ARCH),amd64) USE_MSVC=true else # To build 32-bit under MSVC, un-comment this line (default is gcc) -#USE_MSVC=true +USE_MSVC=true endif CDEFINES=-DHAVE_PROTECTION -DPSAPI_VERSION=1 -DFFI_BUILDING diff --git a/native/callback.c b/native/callback.c index 522dac0e08..a4ccf5e389 100644 --- a/native/callback.c +++ b/native/callback.c @@ -108,7 +108,7 @@ create_callback(JNIEnv* env, jobject obj, jobject method, jsize argc; JavaVM* vm; int rtype; - char msg[64]; + char msg[MSG_SIZE]; int i; int cvt = 0; const char* throw_type = NULL; @@ -644,8 +644,7 @@ callback_dispatch(ffi_cif* cif, void* resp, void** cbargs, void* user_data) { } tls = get_thread_storage(env); if (tls) { - strncpy(tls->name, args.name ? args.name : "", sizeof(tls->name)); - tls->name[sizeof(tls->name)-1] = 0; + snprintf(tls->name, sizeof(tls->name), "%s", args.name ? args.name : ""); tls->detach = detach; tls->jvm_thread = JNI_FALSE; } diff --git a/native/dispatch.c b/native/dispatch.c index 3701484038..8270a79b1f 100644 --- a/native/dispatch.c +++ b/native/dispatch.c @@ -103,13 +103,13 @@ extern "C" { #ifdef _WIN32 static char* -w32_format_error(int error, char* buf, int len) { +w32_format_error(int err, char* buf, int len) { wchar_t* wbuf = NULL; int wlen = FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM |FORMAT_MESSAGE_IGNORE_INSERTS |FORMAT_MESSAGE_ALLOCATE_BUFFER, - NULL, error, 0, (LPWSTR)&wbuf, 0, NULL); + NULL, err, 0, (LPWSTR)&wbuf, 0, NULL); if (wlen > 0) { int result = WideCharToMultiByte(CP_UTF8, 0, wbuf, -1, buf, len, NULL, NULL); if (result == 0) { @@ -127,6 +127,7 @@ w32_format_error(int error, char* buf, int len) { if (wbuf) { LocalFree(wbuf); } + return buf; } static HANDLE @@ -355,7 +356,7 @@ throwByName(JNIEnv *env, const char *name, const char *msg) /** Translate FFI errors into exceptions. */ jboolean ffi_error(JNIEnv* env, const char* op, ffi_status status) { - char msg[256]; + char msg[MSG_SIZE]; switch(status) { case FFI_BAD_ABI: snprintf(msg, sizeof(msg), "%s: Invalid calling convention", op); @@ -394,7 +395,7 @@ dispatch(JNIEnv *env, void* func, jint flags, jobjectArray arr, void** ffi_values; ffi_abi abi; ffi_status status; - char msg[128]; + char msg[MSG_SIZE]; callconv_t callconv = flags & MASK_CC; const char* volatile throw_type = NULL; const char* volatile throw_msg = NULL; @@ -576,11 +577,11 @@ dispatch(JNIEnv *env, void* func, jint flags, jobjectArray arr, } ffi_call(&cif, FFI_FN(func), resP, ffi_values); { - int error = GET_LAST_ERROR(); - JNA_set_last_error(env, error); - if ((flags & THROW_LAST_ERROR) && error) { - char emsg[1024]; - snprintf(msg, sizeof(msg), "[%d] %s", error, STR_ERROR(error, emsg, sizeof(emsg))); + int err = GET_LAST_ERROR(); + JNA_set_last_error(env, err); + if ((flags & THROW_LAST_ERROR) && err) { + char emsg[MSG_SIZE]; + snprintf(msg, sizeof(msg), "[%d] %s", err, STR_ERROR(err, emsg, sizeof(emsg))); throw_type = ELastError; throw_msg = msg; } @@ -1614,7 +1615,7 @@ method_handler(ffi_cif* cif, void* volatile resp, void** argp, void *cdata) { void* oldresp = resp; const char* volatile throw_type = NULL; const char* volatile throw_msg = NULL; - char msg[64]; + char msg[MSG_SIZE]; if (data->flags) { objects = alloca(data->cif.nargs * sizeof(void*)); @@ -1753,16 +1754,15 @@ method_handler(ffi_cif* cif, void* volatile resp, void** argp, void *cdata) { } ffi_call(&data->cif, FFI_FN(data->fptr), resp, args); { - int error = GET_LAST_ERROR(); - JNA_set_last_error(env, error); - if (data->throw_last_error && error) { - char emsg[1024]; - snprintf(msg, sizeof(msg), "[%d] %s", error, STR_ERROR(error, emsg, sizeof(emsg))); + int err = GET_LAST_ERROR(); + JNA_set_last_error(env, err); + if (data->throw_last_error && err) { + char emsg[MSG_SIZE]; + snprintf(msg, sizeof(msg), "[%d] %s", err, STR_ERROR(err, emsg, sizeof(emsg))); throw_type = ELastError; throw_msg = msg; } } - PROTECTED_END(do { throw_type=EError;throw_msg="Invalid memory access"; } while(0)); } @@ -2598,7 +2598,7 @@ Java_com_sun_jna_Native_sizeof(JNIEnv *env, jclass UNUSED(cls), jint type) case com_sun_jna_Native_TYPE_SIZE_T: return sizeof(size_t); default: { - char msg[1024]; + char msg[MSG_SIZE]; snprintf(msg, sizeof(msg), "Invalid sizeof type %d", (int)type); throwByName(env, EIllegalArgument, msg); return -1; @@ -2868,12 +2868,12 @@ Java_com_sun_jna_Native_getWindowHandle0(JNIEnv *env, jclass UNUSED(classp), job #define JAWT_NAME path #endif if ((jawt_handle = LOAD_LIBRARY(JAWT_NAME, DEFAULT_LOAD_OPTS)) == NULL) { - char msg[1024]; + char msg[MSG_SIZE]; throwByName(env, EUnsatisfiedLink, LOAD_ERROR(msg, sizeof(msg))); return -1; } if ((pJAWT_GetAWT = (void*)FIND_ENTRY(jawt_handle, METHOD_NAME)) == NULL) { - char msg[1024], buf[1024]; + char msg[MSG_SIZE], buf[MSG_SIZE]; snprintf(msg, sizeof(msg), "Error looking up JAWT method %s: %s", METHOD_NAME, LOAD_ERROR(buf, sizeof(buf))); throwByName(env, EUnsatisfiedLink, msg); diff --git a/native/dispatch.h b/native/dispatch.h index 9adfd05214..99e10c9286 100644 --- a/native/dispatch.h +++ b/native/dispatch.h @@ -13,6 +13,8 @@ #ifndef DISPATCH_H #define DISPATCH_H +#define MSG_SIZE 1024 + #include "ffi.h" #include "com_sun_jna_Function.h" #include "com_sun_jna_Native.h" @@ -130,6 +132,17 @@ typedef struct _callback { #endif #if defined(_MSC_VER) +// snprintf on windows is broken; always nul-terminate manually +// DO NOT rely on the return value... +static int snprintf(char * str, size_t size, const char * format, ...) { + int retval; + va_list ap; + va_start(ap, format); + retval = _vsnprintf_s(str, size, _TRUNCATE, format, ap); + va_end(ap); + return retval; +} +#define strdup _strdup #if defined(_WIN64) #define L2A(X) ((void *)(X)) #define A2L(X) ((jlong)(X)) @@ -137,12 +150,6 @@ typedef struct _callback { #define L2A(X) ((void *)(unsigned long)(X)) #define A2L(X) ((jlong)(unsigned long)(X)) #endif -#define snprintf sprintf_s -#define strdup _strdup -#else -#if defined(_WIN32_WCE) -#define snprintf _snprintf -#endif #endif /* Convenience macros */ diff --git a/native/testlib.c b/native/testlib.c index 7ac66d320d..8314729033 100644 --- a/native/testlib.c +++ b/native/testlib.c @@ -660,10 +660,12 @@ EXPORT void callVoidCallbackThreaded(void (*func)(void), int n, int ms, const char* name) { THREAD_T thread; thread_data* data = (thread_data*)malloc(sizeof(thread_data)); + size_t len = strlen(name); data->repeat_count = n; data->sleep_time = ms; data->func = func; - snprintf(data->name, sizeof(data->name), "%s", name); + memcpy(data->name, name, len < sizeof(data->name) ? len + 1 : sizeof(data->name)); + data->name[sizeof(data->name)-1] = 0; THREAD_CREATE(&thread, &thread_function, data); } diff --git a/native/wrap_vsnprintf.c b/native/wrap_vsnprintf.c new file mode 100755 index 0000000000..6792aaf575 --- /dev/null +++ b/native/wrap_vsnprintf.c @@ -0,0 +1,131 @@ +#include +#include +#include +#include +#include + +#include + +#define BUFSIZE_0 1024 + +#ifdef _UNICODE +#define __VSNTPRINTF__ vsnwprintf +#define __SNTPRINTF__ snwprintf +#else +#define __VSNTPRINTF__ vsnprintf +#define __SNTPRINTF__ snprintf +#endif + +int +__VSNTPRINTF__ (_TCHAR* s, size_t n, const _TCHAR *format, va_list ap) +{ + int res; + _TCHAR* tmpbuf; + size_t bufsize = n; + + /* If the format string is empty, nothing to do. */ + if (__builtin_expect ((strlen (format) == 0), 0)) + return 0; + + /* The supplied count may be big enough. Try to use the library + _vsntprintf, fixing up the case where the library function + neglects to terminate with '/0'. */ + if (n > 0) + { + /* A NULL destination will cause a segfault with _vsnprintf. + if n > 0. Nor do we want to copy our tmpbuf to NULL later. */ + if (!s) + return -1; + res = _vsntprintf (s, n, format, ap); + if (res > 0) + { + if ((unsigned) res == n) + s[res - 1] = 0; + return res; + } + /* If n is already larger than INT_MAX, increasing it won't + help. */ + if (n > INT_MAX) + return -1; + + /* Try a larger buffer. */ + bufsize *= 2; + } + + if (bufsize < BUFSIZE_0) + bufsize = BUFSIZE_0; + tmpbuf = (_TCHAR *) malloc (bufsize * sizeof (_TCHAR)); + if (!tmpbuf) + return -1; + + res = _vsntprintf (tmpbuf, bufsize, format, ap); + + /* The test for bufsize limit is probably not necesary + with 2GB address space iimit, since, in practice, malloc will + fail well before INT_MAX. */ + while (res < 0 && bufsize <= INT_MAX) + { + _TCHAR * newbuf; + bufsize *= 2; + newbuf = (_TCHAR*) realloc (tmpbuf, bufsize * sizeof (_TCHAR)); + if (!newbuf) + break; + tmpbuf = newbuf; + res = _vsntprintf (tmpbuf, bufsize, format,ap); + } + + if (res > 0 && n > 0) + { + if (n > (unsigned) res) + memcpy (s, tmpbuf, (res + 1) * sizeof (_TCHAR)); + else + { + memcpy (s, tmpbuf, (n - 1) * sizeof (_TCHAR)); + s[n - 1] = 0; + } + } + + free (tmpbuf); + return res; +} + +int __SNTPRINTF__(_TCHAR* s, size_t n, const _TCHAR* format, ...) +{ + int res; + va_list ap; + + va_start (ap, format); + res = __VSNTPRINTF__ (s, n, format, ap); + va_end (ap); + return res; +} + +#ifdef TEST +int main(void) +{ +char string[10]; +char string2[10]; +char string3[12]; +char bigstring [1024 * 8]; +int rv; + +rv = snprintf(string, sizeof(string), "%s", "longer than ten"); +printf("[%s] (%d)\n", string, rv); + +rv = snprintf(string2, sizeof(string2), "%s", "shorter"); +printf("[%s] (%d)\n", string2, rv); + +rv = snprintf(string3, sizeof(string3), "%s%d%s", "longer", 7777, "than ten"); +printf("[%s] (%d)\n", string3, rv); + +rv = snprintf(string, 0, "%s%d%s", "longer", 7777, "than ten"); +printf("[%s] (%d)\n", string, rv); + +memset (bigstring, 'x', sizeof (bigstring)); +bigstring [sizeof (bigstring) - 1] = 0; +rv = snprintf(NULL, 0, "%s", bigstring); +printf("[%s] (%d)\n", string, rv); + +return 0; +} +#endif