I just reviewed and validated the fix provided by Evgeny Kapun. And it looks
fine !!
I thought to share some description of this defect.
Please consider the code in
File: hotspot/src/share/vm/classfile/systemDictionary.cpp
Function: SystemDictionary::resolve_from_stream
Code snippet:
---------------------
TempNewSymbol parsed_name = NULL;
// Parse the stream. Note that we do this even though this klass might
// already be present in the SystemDictionary, otherwise we would not
// throw potential ClassFormatErrors.
//
// Note: "name" is updated.
instanceKlassHandle k = ClassFileParser(st).parseClassFile(class_name,
loader_data,
protection_domain,
parsed_name,
verify,
THREAD);
const char* pkg = "java/";
if (!HAS_PENDING_EXCEPTION &&
!class_loader.is_null() &&
parsed_name != NULL &&
!strncmp((const char*)parsed_name->bytes(), pkg, strlen(pkg))) {
//<---------- (A)
// It is illegal to define classes in the "java." package from
// JVM_DefineClass or jni_DefineClass unless you're the bootclassloader
ResourceMark rm(THREAD);
char* name = parsed_name->as_C_string();
char* index = strrchr(name, '/');
*index = '\0'; // chop to just the package name
while ((index = strchr(name, '/')) != NULL) {
*index = '.'; // replace '/' with '.' in package name
}
const char* fmt = "Prohibited package name: %s";
size_t len = strlen(fmt) + strlen(name);
char* message = NEW_RESOURCE_ARRAY(char, len);
jio_snprintf(message, len, fmt, name);
Exceptions::_throw_msg(THREAD_AND_LOCATION,
vmSymbols::java_lang_SecurityException(), message);
}
------------------------------
The reason of the defect is the "strncmp" call marked as (A).
The condition was added to detect a "parsed_name" starting with "java/". This
is to avoid users defining classes in "java.".
Please note the arguments to "strncmp" are
First argument: Character sequence (Not terminated with NULL but defined with
length parsed_name->utf8_length())
Second argument: Character array (Null terminated character string, length can
be found using "strlen")
Third argument: Length of second argument.
Please note that "strncmp" call works well
-when both the strings to be compared are NULL
terminated character sequence (string)
-Or, The length of character sequence (not
terminating with NULL) is greater than the length of NULL terminated string.
The case in consideration due to implementation of name storage logic. The
names are stored in character array buffer. This string is not terminated with
NULL, but are defined with length stored in another field of data structure.
But for other cases we end up comparing characters beyond the actual length of
first argument. That can produce unpredictable results.
Example:
String "java" is stored as (name part of java.java)
Length=4,location -->|j|a|v|a|<next char>|<next char>|
In our failed case strncmp, compares 5 characters, the fifth character can be
anything if it happens to be fifth character of second string ('/') then the
code would report wrong match.
But for other cases we end up comparing characters beyond the actual length of
first argument. That can produce unpredictable results.
Patch suggested by submitter is
--------------------------
--- a/hotspot/src/share/vm/classfile/systemDictionary.cpp
+++ b/hotspot/src/share/vm/classfile/systemDictionary.cpp
@@ -1087,6 +1087,7 @@
if (!HAS_PENDING_EXCEPTION &&
!class_loader.is_null() &&
parsed_name != NULL &&
+ parsed_name->utf8_length() >= strlen(pkg) &&
!strncmp((const char*)parsed_name->bytes(), pkg, strlen(pkg))) {
// It is illegal to define classes in the "java." package from
// JVM_DefineClass or jni_DefineClass unless you're the bootclassloade
--------------------------
Additional check solves the issue.
Regards,
Alok Kumar Sharma
Hewlett Packard Enterprise
