src/share/vm/classfile/classFileParser.cpp

rev 1449 : 6930553: classfile format checker allows invalid method descriptor in CONSTANT_NameAndType_info in some cases
Summary: Check NameAndType_info signatures aggressively, even when unreferenced
Reviewed-by:

*** 23,36 ****
   */
  
  #include "incls/_precompiled.incl"
  #include "incls/_classFileParser.cpp.incl"
  
! // We generally try to create the oops directly when parsing, rather than allocating
! // temporary data structures and copying the bytes twice. A temporary area is only
! // needed when parsing utf8 entries in the constant pool and when parsing line number
! // tables.
  
  // We add assert in debug mode when class format is not checked.
  
  #define JAVA_CLASSFILE_MAGIC              0xCAFEBABE
  #define JAVA_MIN_SUPPORTED_VERSION        45
--- 23,36 ----
   */
  
  #include "incls/_precompiled.incl"
  #include "incls/_classFileParser.cpp.incl"
  
! // We generally try to create the oops directly when parsing, rather than
! // allocating temporary data structures and copying the bytes twice. A
! // temporary area is only needed when parsing utf8 entries in the constant
! // pool and when parsing line number tables.
  
  // We add assert in debug mode when class format is not checked.
  
  #define JAVA_CLASSFILE_MAGIC              0xCAFEBABE
  #define JAVA_MIN_SUPPORTED_VERSION        45
*** 45,55 ****
--- 45,59 ----
  // Used for backward compatibility reasons:
  // - to check for javac bug fixes that happened after 1.5
  // - also used as the max version when running in jdk6
  #define JAVA_6_VERSION                    50
  
+ // Used for backward compatibility reasons:
+ // - to check NameAndType_info signatures more aggressively
+ #define JAVA_7_VERSION                    51
  
+ 
  void ClassFileParser::parse_constant_pool_entries(constantPoolHandle cp, int length, TRAPS) {
    // Use a local copy of ClassFileStream. It helps the C++ compiler to optimize
    // this function (_current can be allocated in a register, with scalar
    // replacement of aggregates). The _current pointer is copied back to
    // stream() when this function returns. DON'T call another method within
*** 382,391 ****
--- 386,409 ----
          symbolHandle class_name(THREAD, cp->unresolved_klass_at(index));
          // check the name, even if _cp_patches will overwrite it
          verify_legal_class_name(class_name, CHECK_(nullHandle));
          break;
        }
+       case JVM_CONSTANT_NameAndType: {
+         if (_need_verify && _major_version >= JAVA_7_VERSION) {
+           int sig_index = cp->signature_ref_index_at(index);
+           int name_index = cp->name_ref_index_at(index);
+           symbolHandle name(THREAD, cp->symbol_at(name_index));
+           symbolHandle sig(THREAD, cp->symbol_at(sig_index));
+           if (sig->byte_at(0) == JVM_SIGNATURE_FUNC) {
+             verify_legal_method_signature(name, sig, CHECK_(nullHandle));
+           } else {
+             verify_legal_field_signature(name, sig, CHECK_(nullHandle));
+           }
+         }
+         break;
+       }
        case JVM_CONSTANT_Fieldref:
        case JVM_CONSTANT_Methodref:
        case JVM_CONSTANT_InterfaceMethodref: {
          int name_and_type_ref_index = cp->name_and_type_ref_index_at(index);
          // already verified to be utf8
*** 394,407 ****
--- 412,443 ----
          int signature_ref_index = cp->signature_ref_index_at(name_and_type_ref_index);
          symbolHandle name(THREAD, cp->symbol_at(name_ref_index));
          symbolHandle signature(THREAD, cp->symbol_at(signature_ref_index));
          if (tag == JVM_CONSTANT_Fieldref) {
            verify_legal_field_name(name, CHECK_(nullHandle));
+           if (_need_verify && _major_version >= JAVA_7_VERSION) {
+             // Signature is verified above, when iterating NameAndType_info.
+             // Need only to be sure it's the right type.
+             if (signature->byte_at(0) == JVM_SIGNATURE_FUNC) {
+               throwIllegalSignature(
+                   "Field", name, signature, CHECK_(nullHandle));
+             }
+           } else {
              verify_legal_field_signature(name, signature, CHECK_(nullHandle));
+           }
          } else {
            verify_legal_method_name(name, CHECK_(nullHandle));
+           if (_need_verify && _major_version >= JAVA_7_VERSION) {
+             // Signature is verified above, when iterating NameAndType_info.
+             // Need only to be sure it's the right type.
+             if (signature->byte_at(0) != JVM_SIGNATURE_FUNC) {
+               throwIllegalSignature(
+                   "Method", name, signature, CHECK_(nullHandle));
+             }
+           } else {
              verify_legal_method_signature(name, signature, CHECK_(nullHandle));
+           }
            if (tag == JVM_CONSTANT_Methodref) {
              // 4509014: If a class method name begins with '<', it must be "<init>".
              assert(!name.is_null(), "method name in constant pool is null");
              unsigned int name_len = name->utf8_length();
              assert(name_len > 0, "bad method name");  // already verified as legal name
*** 1311,1320 ****
--- 1347,1364 ----
                        "Exceptions attribute has wrong length in class file %s", CHECK_NULL);
    }
    return checked_exceptions_start;
  }
  
+ void ClassFileParser::throwIllegalSignature(
+     const char* type, symbolHandle name, symbolHandle sig, TRAPS) {
+   ResourceMark rm(THREAD);
+   Exceptions::fthrow(THREAD_AND_LOCATION,
+       vmSymbols::java_lang_ClassFormatError(),
+       "%s \"%s\" in class %s has illegal signature \"%s\"", type,
+       name->as_C_string(), _class_name->as_C_string(), sig->as_C_string());
+ }
  
  #define MAX_ARGS_SIZE 255
  #define MAX_CODE_SIZE 65535
  #define INITIAL_MAX_LVT_NUMBER 256
  
*** 4056,4073 ****
    char* bytes = signature->as_utf8_flexible_buffer(THREAD, buf, fixed_buffer_size);
    unsigned int length = signature->utf8_length();
    char* p = skip_over_field_signature(bytes, false, length, CHECK);
  
    if (p == NULL || (p - bytes) != (int)length) {
!     ResourceMark rm(THREAD);
!     Exceptions::fthrow(
!       THREAD_AND_LOCATION,
!       vmSymbolHandles::java_lang_ClassFormatError(),
!       "Field \"%s\" in class %s has illegal signature \"%s\"",
!       name->as_C_string(), _class_name->as_C_string(), bytes
!     );
!     return;
    }
  }
  
  // Checks if signature is a legal method signature.
  // Returns number of parameters
--- 4100,4110 ----
    char* bytes = signature->as_utf8_flexible_buffer(THREAD, buf, fixed_buffer_size);
    unsigned int length = signature->utf8_length();
    char* p = skip_over_field_signature(bytes, false, length, CHECK);
  
    if (p == NULL || (p - bytes) != (int)length) {
!     throwIllegalSignature("Field", name, signature, CHECK);
    }
  }
  
  // Checks if signature is a legal method signature.
  // Returns number of parameters
*** 4114,4130 ****
          }
        }
      }
    }
    // Report error
!   ResourceMark rm(THREAD);
!   Exceptions::fthrow(
!     THREAD_AND_LOCATION,
!     vmSymbolHandles::java_lang_ClassFormatError(),
!     "Method \"%s\" in class %s has illegal signature \"%s\"",
!     name->as_C_string(),  _class_name->as_C_string(), p
!   );
    return 0;
  }
  
  
  // Unqualified names may not contain the characters '.', ';', or '/'.
--- 4151,4161 ----
          }
        }
      }
    }
    // Report error
!   throwIllegalSignature("Method", name, signature, CHECK_0);
    return 0;
  }
  
  
  // Unqualified names may not contain the characters '.', ';', or '/'.