8339134: Callers of Exceptions::fthrow should ensure exception message lengths avoid the INT_MAX limits of os::vsnprintf

Reviewed-by: coleenp, jsjolen
This commit is contained in:
David Holmes 2024-11-25 19:55:26 +00:00
parent df2d4c1575
commit 8de158aefe
9 changed files with 49 additions and 6 deletions

View File

@ -34,6 +34,10 @@
PRAGMA_DIAG_PUSH
PRAGMA_FORMAT_NONLITERAL_IGNORED
// None of the error routines below take in a free-form, potentially unbounded
// string, and names are all limited to < 64K, so we know that all formatted
// strings passed to fthrow will not be excessively large.
void ClassFileParser::classfile_parse_error(const char* msg, TRAPS) const {
assert(_class_name != nullptr, "invariant");
ResourceMark rm(THREAD);

View File

@ -1794,6 +1794,7 @@ void ClassFileParser::throwIllegalSignature(const char* type,
assert(sig != nullptr, "invariant");
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(THREAD_AND_LOCATION,
vmSymbols::java_lang_ClassFormatError(),
"%s \"%s\" in class %s has illegal signature \"%s\"", type,
@ -4073,6 +4074,8 @@ void ClassFileParser::check_super_class_access(const InstanceKlass* this_klass,
char* msg = Reflection::verify_class_access_msg(this_klass,
InstanceKlass::cast(super),
vca_result);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
if (msg == nullptr) {
bool same_module = (this_klass->module() == super->module());
Exceptions::fthrow(
@ -4121,6 +4124,8 @@ void ClassFileParser::check_super_interface_access(const InstanceKlass* this_kla
char* msg = Reflection::verify_class_access_msg(this_klass,
k,
vca_result);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
if (msg == nullptr) {
bool same_module = (this_klass->module() == k->module());
Exceptions::fthrow(
@ -4217,6 +4222,8 @@ static void check_illegal_static_method(const InstanceKlass* this_klass, TRAPS)
// if m is static and not the init method, throw a verify error
if ((m->is_static()) && (m->name() != vmSymbols::class_initializer_name())) {
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_VerifyError(),
@ -4236,6 +4243,7 @@ void ClassFileParser::verify_legal_class_modifiers(jint flags, TRAPS) const {
assert(_major_version >= JAVA_9_VERSION || !is_module, "JVM_ACC_MODULE should not be set");
if (is_module) {
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_NoClassDefFoundError(),
@ -4259,6 +4267,7 @@ void ClassFileParser::verify_legal_class_modifiers(jint flags, TRAPS) const {
(is_interface && major_gte_1_5 && (is_super || is_enum)) ||
(!is_interface && major_gte_1_5 && is_annotation)) {
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_ClassFormatError(),
@ -4295,6 +4304,7 @@ void ClassFileParser::verify_class_version(u2 major, u2 minor, Symbol* class_nam
}
if (major > max_version) {
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_UnsupportedClassVersionError(),
@ -4310,6 +4320,7 @@ void ClassFileParser::verify_class_version(u2 major, u2 minor, Symbol* class_nam
if (minor == JAVA_PREVIEW_MINOR_VERSION) {
if (major != max_version) {
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_UnsupportedClassVersionError(),
@ -4362,6 +4373,7 @@ void ClassFileParser::verify_legal_field_modifiers(jint flags,
if (is_illegal) {
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_ClassFormatError(),
@ -4445,6 +4457,7 @@ void ClassFileParser::verify_legal_method_modifiers(jint flags,
if (is_illegal) {
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_ClassFormatError(),
@ -4686,6 +4699,7 @@ void ClassFileParser::verify_legal_class_name(const Symbol* name, TRAPS) const {
if (!legal) {
ResourceMark rm(THREAD);
assert(_class_name != nullptr, "invariant");
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_ClassFormatError(),
@ -4719,6 +4733,7 @@ void ClassFileParser::verify_legal_field_name(const Symbol* name, TRAPS) const {
if (!legal) {
ResourceMark rm(THREAD);
assert(_class_name != nullptr, "invariant");
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_ClassFormatError(),
@ -4756,6 +4771,7 @@ void ClassFileParser::verify_legal_method_name(const Symbol* name, TRAPS) const
if (!legal) {
ResourceMark rm(THREAD);
assert(_class_name != nullptr, "invariant");
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_ClassFormatError(),
@ -5527,6 +5543,7 @@ void ClassFileParser::parse_stream(const ClassFileStream* const stream,
if (_class_name != class_name_in_cp) {
if (_class_name != vmSymbols::unknown_class_name()) {
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(THREAD_AND_LOCATION,
vmSymbols::java_lang_NoClassDefFoundError(),
"%s (wrong name: %s)",

View File

@ -323,6 +323,9 @@ void LinkResolver::check_klass_accessibility(Klass* ref_klass, Klass* sel_klass,
char* msg = Reflection::verify_class_access_msg(ref_klass,
InstanceKlass::cast(base_klass),
vca_result);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
bool same_module = (base_klass->module() == ref_klass->module());
if (msg == nullptr) {
Exceptions::fthrow(
@ -615,6 +618,7 @@ void LinkResolver::check_method_accessability(Klass* ref_klass,
print_nest_host_error_on(&ss, ref_klass, sel_klass);
}
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(THREAD_AND_LOCATION,
vmSymbols::java_lang_IllegalAccessError(),
"%s",
@ -968,6 +972,7 @@ void LinkResolver::check_field_accessability(Klass* ref_klass,
if (fd.is_private()) {
print_nest_host_error_on(&ss, ref_klass, sel_klass);
}
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(THREAD_AND_LOCATION,
vmSymbols::java_lang_IllegalAccessError(),
"%s",
@ -1187,6 +1192,7 @@ Method* LinkResolver::linktime_resolve_special_method(const LinkInfo& link_info,
ss.print(" %s(", resolved_method->name()->as_C_string());
resolved_method->signature()->print_as_signature_external_parameters(&ss);
ss.print(")' not found");
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_NoSuchMethodError(),

View File

@ -1266,6 +1266,7 @@ oop ConstantPool::resolve_constant_at_impl(const constantPoolHandle& this_cp,
cp_index,
callee->is_interface() ? "CONSTANT_MethodRef" : "CONSTANT_InterfaceMethodRef",
callee->is_interface() ? "CONSTANT_InterfaceMethodRef" : "CONSTANT_MethodRef");
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(THREAD_AND_LOCATION, vmSymbols::java_lang_IncompatibleClassChangeError(), "%s", ss.as_string());
save_and_throw_exception(this_cp, cp_index, tag, CHECK_NULL);
}

View File

@ -899,6 +899,7 @@ bool InstanceKlass::link_class_impl(TRAPS) {
// if we are executing Java code. This is not a problem for CDS dumping phase since
// it doesn't execute any Java code.
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(THREAD_AND_LOCATION,
vmSymbols::java_lang_NoClassDefFoundError(),
"Class %s, or one of its supertypes, failed class initialization",
@ -919,6 +920,7 @@ bool InstanceKlass::link_class_impl(TRAPS) {
if (super_klass != nullptr) {
if (super_klass->is_interface()) { // check if super class is an interface
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_IncompatibleClassChangeError(),
@ -3286,6 +3288,7 @@ InstanceKlass* InstanceKlass::compute_enclosing_class(bool* inner_is_member, TRA
// If the outer class is not an instance klass then it cannot have
// declared any inner classes.
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_IncompatibleClassChangeError(),

View File

@ -697,6 +697,7 @@ void Reflection::check_for_inner_class(const InstanceKlass* outer, const Instanc
// 'inner' not declared as an inner klass in outer
ResourceMark rm(THREAD);
// Names are all known to be < 64k so we know this formatted message is not excessively large.
Exceptions::fthrow(
THREAD_AND_LOCATION,
vmSymbols::java_lang_IncompatibleClassChangeError(),

View File

@ -115,12 +115,13 @@ template <> void DCmdArgument<jlong>::parse_value(const char* str,
|| sscanf(str, JLONG_FORMAT "%n", &_value, &scanned) != 1
|| (size_t)scanned != len)
{
const int maxprint = 64;
Exceptions::fthrow(THREAD_AND_LOCATION, vmSymbols::java_lang_IllegalArgumentException(),
"Integer parsing error in command argument '%s'. Could not parse: %.*s%s.\n", _name,
MIN2((int)len, maxprint),
(str == nullptr ? "<null>" : str),
(len > maxprint ? "..." : ""));
"Integer parsing error in command argument '%.*s'. Could not parse: %.*s%s.\n",
maxprint,
_name,
maxprint > len ? (int)len : maxprint,
(str == nullptr ? "<null>" : str),
(len > maxprint ? "..." : ""));
}
}
@ -160,7 +161,11 @@ PRAGMA_DIAG_POP
buf[len] = '\0';
Exceptions::fthrow(THREAD_AND_LOCATION, vmSymbols::java_lang_IllegalArgumentException(),
"Boolean parsing error in command argument '%s'. Could not parse: %s.\n", _name, buf);
"Boolean parsing error in command argument '%.*s'. Could not parse: %.*s.\n",
maxprint,
_name,
maxprint,
buf);
}
}
}

View File

@ -62,6 +62,7 @@ public:
class GenDCmdArgument : public ResourceObj {
protected:
static const int maxprint = 64;
GenDCmdArgument* _next;
const char* const _name;
const char* const _description;

View File

@ -258,6 +258,10 @@ void Exceptions::throw_stack_overflow_exception(JavaThread* THREAD, const char*
_throw(THREAD, file, line, exception);
}
// All callers are expected to have ensured that the incoming expanded format string
// will be within reasonable limits - specifically we will never hit the INT_MAX limit
// of os::vsnprintf when it tries to report how big a buffer is needed. Even so we
// further limit the formatted output to 1024 characters.
void Exceptions::fthrow(JavaThread* thread, const char* file, int line, Symbol* h_name, const char* format, ...) {
const int max_msg_size = 1024;
va_list ap;
@ -273,6 +277,7 @@ void Exceptions::fthrow(JavaThread* thread, const char* file, int line, Symbol*
// have a truncated UTF-8 sequence. Similarly, if the buffer was too small and ret >= max_msg_size
// we may also have a truncated UTF-8 sequence. In such cases we need to fix the buffer so the UTF-8
// sequence is valid.
assert(ret != -1, "Caller should have ensured the incoming format string is size limited!");
if (ret == -1 || ret >= max_msg_size) {
int len = (int) strlen(msg);
if (len > 0) {