On 5/29/20 4:31 PM, Johannes Kuhn wrote:
Thanks.

Noted that readInt can read outside of the byte array when it is shorter than 4 bytes, throwing an ArrayIndexOutOfBoundsException.
Same applies for readUnsignedShort.

Test cases for malformedClassFiles:
new byte[3], new byte[] {0xCA, 0xFE, 0xBA, 0xBE, 0x00}


Oops.  My bad.  The range check should be fixed to:

   if ((offset+4) > bytes.length) {
       throw new ClassFormatError("Invalid ClassFile structure");
   }

similarly the check for readUnsignedShort is:
    if ((offset+2) > bytes.length)

Mandy
- Johannes


On 29-May-20 21:23, Mandy Chung wrote:
It's fixed.  Thanks
Mandy

On 5/28/20 4:35 PM, Johannes Kuhn wrote:
Hi,

just noticed a small thing:
The magic is 4 bytes, but readUnsignedShort reads two bytes.

- Johannes

On 28-May-20 19:25, Mandy Chung wrote:


On 5/28/20 6:55 AM, Alan Bateman wrote:
On 28/05/2020 05:13, Mandy Chung wrote:
Updated webrev:
http://cr.openjdk.java.net/~mchung/jdk15/webrevs/8245432/webrev.01/

I modify this patch to check the class file version and throws CFE if unsupported before creating ClassReader. This also fixes JDK-8245061 that it reads the value of `this_class` as a constant (as ASM will throw an exception if it's not a constant) and also ensures that `this_class` is a CONSTANT_Class_info by checking the descriptor string from Type.
I don't want to complicate this further but the --enable-preview handling in the VM doesn't seem to expose an internal property that allows code in the libraries know if preview feature are enabled or not. I was assuming that isSupportedClassFileVersion would need to check that.


The runtime will ensure if --enable-preview is set if a class file with minor is loaded.   I will prefer to keep VM::isSupportedClassFileVersion to validate the given major/minor version.  At runtime, it will fail when such class file is loaded if preview is not enabled.

I'll add a comment to describe that.

Will readUnsignedShort throw AIOBE if the offset is beyond the length?

Good catch.  I add the check to throw CFE.
+            private static int readUnsignedShort(byte[] bytes, int offset) {
+                if (offset >= bytes.length) {
+                    throw new ClassFormatError("Invalid ClassFile structure");
+                }
+                return ((bytes[offset] & 0xFF) << 8) | (bytes[offset + 1] & 0xFF);
+            }

Also are you sure about "& 0xCAFEBABE", I assumed it would just check the magic number is equal to that.

It should check ==.  Fixed.

thanks
Mandy





Reply via email to