Origin: http://icedtea.classpath.org/hg/release/icedtea-web-1.2/rev/d7375e2a9076 and http://icedtea.classpath.org/hg/release/icedtea-web-1.2/rev/d65bd94e0ba9 Subject: incorrect handling of not 0-terminated strings # HG changeset patch # User Deepak Bhole # Date 1338581118 14400 # Node ID d7375e2a907697655e5b3051b1bf901256b2c73b # Parent 0cdefd555de730965406802416044b4fb5ef02eb Fixed PR863: Error passing strings to applet methods in Chromium PR863: Error passing strings to applet methods in Chromium * plugin/icedteanp/IcedTeaJavaRequestProcessor.cc (createJavaObjectFromVariant): Account for length of the characters. * plugin/icedteanp/IcedTeaNPPlugin.cc (plugin_get_documentbase): Same. * plugin/icedteanp/IcedTeaPluginRequestProcessor.cc (_eval): Print the string's c_str rather than utf8characters/ * plugin/icedteanp/IcedTeaPluginUtils.cc (printNPVariant): Account for length of the characters. (NPVariantToString): Same. (isObjectJSArray): Same. # HG changeset patch # User Adam Domurad # Date 1339177465 14400 # Node ID d65bd94e0ba9b7c8b9051c7d471b55c2c74ea3f4 # Parent d7375e2a907697655e5b3051b1bf901256b2c73b fixes PR518, ensures null termination of strings based off of NPVariant results. This patch fixes PR518, ensures null termination of strings based off of NPVariant results. * NEWS: Added line about fixing PR518 * plugin/icedteanp/IcedTeaPluginUtils.h: Added declaration of NPVariantAsString * plugin/icedteanp/IcedTeaPluginUtils.cc (NPVariantAsString): New. Converts an NPVariant to a std::string, assumes it is a string. (isObjectJSArray): Now uses NPVariantAsString, minor cleanup. * plugin/icedteanp/IcedTeaJavaRequestProcessor.cc (plugin_get_documentbase): Now uses NPVariantAsString. * plugin/icedteanp/IcedTeaNPPlugin.cc (NPVariantToString): Now uses NPVariantAsString, minor cleanup. It was discovered that the IcedTea-Web web browser plugin incorrectly assumed that all strings provided by browser are NUL terminated, which is not guaranteed by the NPAPI (Netscape Plugin Application Programming Interface). When used in a browser that does not NUL terminate NPVariant NPStrings, this could lead to buffer over-read or over-write, resulting in possible information leak, crash, or code execution. Mozilla browsers currently NUL terminate strings, however recent Chrome versions are known not to provide NUL terminated data. CVE-2012-3423 [Ubuntu note: patch differs from upstream commits in that they have been merged into a single patch and the Changelog and NEWS entries have been pulled out to reduce patch conflicts. --sbeattie] --- plugin/icedteanp/IcedTeaJavaRequestProcessor.cc | 6 -- plugin/icedteanp/IcedTeaNPPlugin.cc | 9 +--- plugin/icedteanp/IcedTeaPluginRequestProcessor.cc | 4 - plugin/icedteanp/IcedTeaPluginUtils.cc | 45 ++++++++++------------ plugin/icedteanp/IcedTeaPluginUtils.h | 3 + 5 files changed, 32 insertions(+), 35 deletions(-) Index: b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc =================================================================== --- a/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc +++ b/plugin/icedteanp/IcedTeaJavaRequestProcessor.cc @@ -904,11 +904,7 @@ createJavaObjectFromVariant(NPP instance } else if (NPVARIANT_IS_STRING(variant)) { className = "java.lang.String"; -#if MOZILLA_VERSION_COLLAPSED < 1090200 - stringArg += NPVARIANT_TO_STRING(variant).utf8characters; -#else - stringArg += NPVARIANT_TO_STRING(variant).UTF8Characters; -#endif + stringArg = IcedTeaPluginUtilities::NPVariantAsString(variant); } else if (NPVARIANT_IS_OBJECT(variant)) { Index: b/plugin/icedteanp/IcedTeaNPPlugin.cc =================================================================== --- a/plugin/icedteanp/IcedTeaNPPlugin.cc +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc @@ -1098,12 +1098,10 @@ plugin_get_documentbase (NPP instance) browser_functions.getproperty(instance, NPVARIANT_TO_OBJECT(location), href_id, &href); + std::string href_str = IcedTeaPluginUtilities::NPVariantAsString(href); + // Strip everything after the last "/" -#if MOZILLA_VERSION_COLLAPSED < 1090200 - gchar** parts = g_strsplit (NPVARIANT_TO_STRING(href).utf8characters, "/", -1); -#else - gchar** parts = g_strsplit (NPVARIANT_TO_STRING(href).UTF8Characters, "/", -1); -#endif + gchar** parts = g_strsplit (href_str.c_str(), "/", -1); guint parts_sz = g_strv_length (parts); std::string location_str; @@ -1118,6 +1116,7 @@ plugin_get_documentbase (NPP instance) // Release references. browser_functions.releasevariantvalue(&href); browser_functions.releasevariantvalue(&location); + g_strfreev(parts); cleanup_done: PLUGIN_DEBUG ("plugin_get_documentbase return\n"); PLUGIN_DEBUG("plugin_get_documentbase returning: %s\n", documentbase_copy); Index: b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc =================================================================== --- a/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc +++ b/plugin/icedteanp/IcedTeaPluginRequestProcessor.cc @@ -842,12 +842,12 @@ _eval(void* data) script.utf8characters = script_str->c_str(); script.utf8length = script_str->size(); - PLUGIN_DEBUG("Evaluating: %s\n", script.utf8characters); + PLUGIN_DEBUG("Evaluating: %s\n", script_str->c_str()); #else script.UTF8Characters = script_str->c_str(); script.UTF8Length = script_str->size(); - PLUGIN_DEBUG("Evaluating: %s\n", script.UTF8Characters); + PLUGIN_DEBUG("Evaluating: %s\n", script_str->c_str()); #endif ((AsyncCallThreadData*) data)->call_successful = browser_functions.evaluate(instance, window_ptr, &script, eval_variant); Index: b/plugin/icedteanp/IcedTeaPluginUtils.cc =================================================================== --- a/plugin/icedteanp/IcedTeaPluginUtils.cc +++ b/plugin/icedteanp/IcedTeaPluginUtils.cc @@ -668,11 +668,8 @@ IcedTeaPluginUtilities::printNPVariant(N } else if (NPVARIANT_IS_STRING(variant)) { -#if MOZILLA_VERSION_COLLAPSED < 1090200 - PLUGIN_DEBUG("STRING: %s\n", NPVARIANT_TO_STRING(variant).utf8characters); -#else - PLUGIN_DEBUG("STRING: %s\n", NPVARIANT_TO_STRING(variant).UTF8Characters); -#endif + std::string str = IcedTeaPluginUtilities::NPVariantAsString(variant); + PLUGIN_DEBUG("STRING: %s (length=%d)\n", str.c_str(), str.size()); } else { @@ -684,7 +681,7 @@ void IcedTeaPluginUtilities::NPVariantToString(NPVariant variant, std::string* result) { char* str = (char*) malloc(sizeof(char)*32); // enough for everything except string - + bool was_string_already = false; if (NPVARIANT_IS_VOID(variant)) { sprintf(str, "%p", variant); @@ -710,21 +707,15 @@ IcedTeaPluginUtilities::NPVariantToStrin } else if (NPVARIANT_IS_STRING(variant)) { - free(str); -#if MOZILLA_VERSION_COLLAPSED < 1090200 - str = (char*) malloc(sizeof(char)*NPVARIANT_TO_STRING(variant).utf8length); - sprintf(str, "%s", NPVARIANT_TO_STRING(variant).utf8characters); -#else - str = (char*) malloc(sizeof(char)*NPVARIANT_TO_STRING(variant).UTF8Length); - sprintf(str, "%s", NPVARIANT_TO_STRING(variant).UTF8Characters); -#endif + result->append(IcedTeaPluginUtilities::NPVariantAsString(variant)); + was_string_already = true; } else { sprintf(str, "[Object %p]", variant); } - - result->append(str); + if (!was_string_already) + result->append(str); free(str); } @@ -864,13 +855,7 @@ IcedTeaPluginUtilities::isObjectJSArray( browser_functions.invoke(instance, constructor, toString, NULL, 0, &constructor_str); IcedTeaPluginUtilities::printNPVariant(constructor_str); - std::string constructor_name = std::string(); - -#if MOZILLA_VERSION_COLLAPSED < 1090200 - constructor_name.append(NPVARIANT_TO_STRING(constructor_str).utf8characters); -#else - constructor_name.append(NPVARIANT_TO_STRING(constructor_str).UTF8Characters); -#endif + std::string constructor_name = IcedTeaPluginUtilities::NPVariantAsString(constructor_str); PLUGIN_DEBUG("Constructor for NPObject is %s\n", constructor_name.c_str()); @@ -913,6 +898,20 @@ IcedTeaPluginUtilities::decodeURL(const PLUGIN_DEBUG("SENDING URL: %s\n", *decoded_url); } +/* Copies a variant data type into a C++ string */ +std::string +IcedTeaPluginUtilities::NPVariantAsString(NPVariant variant) +{ +#if MOZILLA_VERSION_COLLAPSED < 1090200 + return std::string(( + NPVARIANT_TO_STRING(variant).utf8characters, + NPVARIANT_TO_STRING(variant).utf8ength); +#else + return std::string( + NPVARIANT_TO_STRING(variant).UTF8Characters, + NPVARIANT_TO_STRING(variant).UTF8Length); +#endif +} /** * Posts a function for execution on the plug-in thread and wait for result. Index: b/plugin/icedteanp/IcedTeaPluginUtils.h =================================================================== --- a/plugin/icedteanp/IcedTeaPluginUtils.h +++ b/plugin/icedteanp/IcedTeaPluginUtils.h @@ -205,6 +205,9 @@ class IcedTeaPluginUtilities /* Converts the given integer to a string */ static void itoa(int i, std::string* result); + /* Copies a variant data type into a C++ string */ + static std::string NPVariantAsString(NPVariant variant); + /* Frees the given vector and the strings that its contents point to */ static void freeStringPtrVector(std::vector* v);