From a6c7b7ea16a0a461c70ffee09c09e54cb002277a Mon Sep 17 00:00:00 2001 From: hseigel Date: Wed, 25 Mar 2015 08:16:48 -0400 Subject: [PATCH] 7127066: Class verifier accepts an invalid class file Summary: For *store bytecodes, compare incoming, not outgoing, type state with exception handlers' stack maps. Reviewed-by: acorn, dholmes --- src/share/vm/classfile/verifier.cpp | 23 ++- src/share/vm/interpreter/bytecodes.hpp | 3 +- test/runtime/stackMapCheck/BadMap.jasm | 152 ++++++++++++++++++ test/runtime/stackMapCheck/BadMapDstore.jasm | 79 +++++++++ test/runtime/stackMapCheck/BadMapIstore.jasm | 79 +++++++++ test/runtime/stackMapCheck/StackMapCheck.java | 63 ++++++++ 6 files changed, 395 insertions(+), 4 deletions(-) create mode 100644 test/runtime/stackMapCheck/BadMap.jasm create mode 100644 test/runtime/stackMapCheck/BadMapDstore.jasm create mode 100644 test/runtime/stackMapCheck/BadMapIstore.jasm create mode 100644 test/runtime/stackMapCheck/StackMapCheck.java diff --git a/src/share/vm/classfile/verifier.cpp b/src/share/vm/classfile/verifier.cpp index 6cd0928ad..ed65f0d78 100644 --- a/src/share/vm/classfile/verifier.cpp +++ b/src/share/vm/classfile/verifier.cpp @@ -655,6 +655,7 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) { bool this_uninit = false; // Set to true when invokespecial initialized 'this' + bool verified_exc_handlers = false; // Merge with the next instruction { @@ -686,6 +687,18 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) { } } + // Look for possible jump target in exception handlers and see if it + // matches current_frame. Do this check here for astore*, dstore*, + // fstore*, istore*, and lstore* opcodes because they can change the type + // state by adding a local. JVM Spec says that the incoming type state + // should be used for this check. So, do the check here before a possible + // local is added to the type state. + if (Bytecodes::is_store_into_local(opcode) && bci >= ex_min && bci < ex_max) { + verify_exception_handler_targets( + bci, this_uninit, ¤t_frame, &stackmap_table, CHECK_VERIFY(this)); + verified_exc_handlers = true; + } + switch (opcode) { case Bytecodes::_nop : no_control_flow = false; break; @@ -1662,9 +1675,13 @@ void ClassVerifier::verify_method(methodHandle m, TRAPS) { } // end switch } // end Merge with the next instruction - // Look for possible jump target in exception handlers and see if it - // matches current_frame - if (bci >= ex_min && bci < ex_max) { + // Look for possible jump target in exception handlers and see if it matches + // current_frame. Don't do this check if it has already been done (for + // ([a,d,f,i,l]store* opcodes). This check cannot be done earlier because + // opcodes, such as invokespecial, may set the this_uninit flag. + assert(!(verified_exc_handlers && this_uninit), + "Exception handler targets got verified before this_uninit got set"); + if (!verified_exc_handlers && bci >= ex_min && bci < ex_max) { verify_exception_handler_targets( bci, this_uninit, ¤t_frame, &stackmap_table, CHECK_VERIFY(this)); } diff --git a/src/share/vm/interpreter/bytecodes.hpp b/src/share/vm/interpreter/bytecodes.hpp index a43033c07..bff8cdc3b 100644 --- a/src/share/vm/interpreter/bytecodes.hpp +++ b/src/share/vm/interpreter/bytecodes.hpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 1997, 2012, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1997, 2015, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -420,6 +420,7 @@ class Bytecodes: AllStatic { static bool is_astore (Code code) { return (code == _astore || code == _astore_0 || code == _astore_1 || code == _astore_2 || code == _astore_3); } + static bool is_store_into_local(Code code){ return (_istore <= code && code <= _astore_3); } static bool is_const (Code code) { return (_aconst_null <= code && code <= _ldc2_w); } static bool is_zero_const (Code code) { return (code == _aconst_null || code == _iconst_0 || code == _fconst_0 || code == _dconst_0); } diff --git a/test/runtime/stackMapCheck/BadMap.jasm b/test/runtime/stackMapCheck/BadMap.jasm new file mode 100644 index 000000000..2a4c7856f --- /dev/null +++ b/test/runtime/stackMapCheck/BadMap.jasm @@ -0,0 +1,152 @@ + /* + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +/* + * This class should throw VerifyError because the StackMap for bytecode index + * 45 (astore_2, line 123) is incorrect. The stack maps for bytecode indexes 45 + * and 49 (astore, line 133) do not match because 45 does not supply enough + * locals to satisfy 49. + * + * The astore_2 bytecode at bytecode index 45 changes the type state, + * preventing the stackmap mismatch. But, if the incoming type state is used, + * as required by JVM Spec 8, then the verifier will detected the stackmap + * mismatch, and throw VerifyError. + */ + +super public class BadMap + version 51:0 +{ + + +public Method "":"()V" + stack 1 locals 1 +{ + aload_0; + invokespecial Method java/lang/Object."":"()V"; + return; +} + +public static Method main:"([Ljava/lang/String;)V" + throws java/lang/Throwable + stack 0 locals 1 +{ + return; +} + +public static Method foo:"()V" + stack 3 locals 5 +{ + iconst_0; + ifne L5; + nop; + try t7; + L5: stack_frame_type full; + aconst_null; + dup; + astore_0; + astore_1; + try t0; + aconst_null; + astore_0; + endtry t0; + goto L19; + catch t0 java/io/IOException; + stack_frame_type full; + locals_map class java/lang/Object, null; + stack_map class java/io/IOException; + astore_2; + aconst_null; + dup; + astore_1; + astore_0; + try t1; + L19: stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object; + aconst_null; + astore_2; + endtry t1; + aload_1; + ifnonnull L37; + nop; + goto L37; + catch t1 #0; + catch t2 #0; + try t2; + stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object; + stack_map class java/lang/Throwable; + astore_3; + endtry t2; + aload_1; + ifnonnull L35; + nop; + L35: stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object, bogus, class java/lang/Throwable; + aload_3; + athrow; + try t3, t4; + L37: stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object; + aload_1; + ifnonnull L42; + nop; + endtry t3, t4; + L42: stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object; + goto L54; + catch t3 java/lang/Exception; + try t5; + stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object; + stack_map class java/lang/Exception; + astore_2; // astore_2, at bci 45, that changes the type state. + endtry t5; + goto L54; + catch t4 #0; + catch t5 #0; + catch t6 #0; + try t6; + stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object; + stack_map class java/lang/Throwable; + astore 4; + endtry t6; + aload 4; + athrow; + L54: stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object; + goto L57; + L57: stack_frame_type full; + locals_map class java/lang/Object, class java/lang/Object, class java/lang/Object; + nop; + endtry t7; + return; + catch t7 #0; + stack_frame_type full; + stack_map class java/lang/Throwable; + nop; + athrow; +} + +} // end Class BadMap diff --git a/test/runtime/stackMapCheck/BadMapDstore.jasm b/test/runtime/stackMapCheck/BadMapDstore.jasm new file mode 100644 index 000000000..16797fb9e --- /dev/null +++ b/test/runtime/stackMapCheck/BadMapDstore.jasm @@ -0,0 +1,79 @@ + /* + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +/* + * This class should throw VerifyError because the StackMap for bytecode index + * 9 (dstore_2, line 60) is incorrect. The stack maps for bytecode indexes 9 + * and 18 (astore_2, line 70) do not match because 9 does not supply enough + * locals to satisfy 18. + * + * The dstore_2 bytecode at bytecode index 9 changes the type state, + * preventing the stackmap mismatch. But, if the incoming type state is used, + * as required by JVM Spec 8, then the verifier will detected the stackmap + * mismatch, and throw VerifyError. + */ + +super public class BadMapDstore + version 51:0 +{ + +Field blah:I; + +public Method "":"()V" + stack 1 locals 1 +{ + aload_0; + invokespecial Method java/lang/Object."":"()V"; + return; +} + +public static Method main:"([Ljava/lang/String;)V" + stack 4 locals 4 +{ + new class BadMapDstore; + dup; + invokespecial Method "":"()V"; + astore_1; + dconst_1; + try t0; + dstore_2; + aload_1; + iconst_5; + putfield Field blah:"I"; + endtry t0; + goto L22; + catch t0 java/lang/Throwable; + stack_frame_type full; + locals_map class "[Ljava/lang/String;", class BadMapDstore, double; + stack_map class java/lang/Throwable; + astore_2; + aload_1; + dconst_0; + dstore_2; + pop; + L22: stack_frame_type same; + return; +} + +} // end Class BadMapDstore diff --git a/test/runtime/stackMapCheck/BadMapIstore.jasm b/test/runtime/stackMapCheck/BadMapIstore.jasm new file mode 100644 index 000000000..63eeaf358 --- /dev/null +++ b/test/runtime/stackMapCheck/BadMapIstore.jasm @@ -0,0 +1,79 @@ + /* + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +/* + * This class should throw VerifyError because the StackMap for bytecode index + * 9 (istore_2, line 60) is incorrect. The stack maps for bytecode indexes 9 + * and 18 (astore_2, line 70) do not match because 9 does not supply enough + * locals to satisfy 18. + * + * The istore_2 bytecode at bytecode index 9 changes the type state, + * preventing the stackmap mismatch. But, if the incoming type state is used, + * as required by JVM Spec 8, then the verifier will detected the stackmap + * mismatch, and throw VerifyError. + */ + +super public class BadMapIstore + version 51:0 +{ + +Field blah:I; + +public Method "":"()V" + stack 1 locals 1 +{ + aload_0; + invokespecial Method java/lang/Object."":"()V"; + return; +} + +public static Method main:"([Ljava/lang/String;)V" + stack 2 locals 3 +{ + new class BadMapIstore; + dup; + invokespecial Method "":"()V"; + astore_1; + iconst_2; + try t0; + istore_2; + aload_1; + iconst_5; + putfield Field blah:"I"; + endtry t0; + goto L22; + catch t0 java/lang/Throwable; + stack_frame_type full; + locals_map class "[Ljava/lang/String;", class BadMapIstore, int; + stack_map class java/lang/Throwable; + astore_2; + aload_1; + iconst_4; + istore_2; + pop; + L22: stack_frame_type same; + return; +} + +} // end Class BadMapIstore diff --git a/test/runtime/stackMapCheck/StackMapCheck.java b/test/runtime/stackMapCheck/StackMapCheck.java new file mode 100644 index 000000000..56d171585 --- /dev/null +++ b/test/runtime/stackMapCheck/StackMapCheck.java @@ -0,0 +1,63 @@ + /* + * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + * + */ + +/* + * @test + * @bug 7127066 + * @summary Class verifier accepts an invalid class file + * @compile BadMap.jasm + * @compile BadMapDstore.jasm + * @compile BadMapIstore.jasm + * @run main/othervm -Xverify:all StackMapCheck + */ + +public class StackMapCheck { + public static void main(String args[]) throws Throwable { + + System.out.println("Regression test for bug 7127066"); + try { + Class newClass = Class.forName("BadMap"); + throw new RuntimeException( + "StackMapCheck failed, BadMap did not throw VerifyError"); + } catch (java.lang.VerifyError e) { + System.out.println("BadMap passed, VerifyError was thrown"); + } + + try { + Class newClass = Class.forName("BadMapDstore"); + throw new RuntimeException( + "StackMapCheck failed, BadMapDstore did not throw VerifyError"); + } catch (java.lang.VerifyError e) { + System.out.println("BadMapDstore passed, VerifyError was thrown"); + } + + try { + Class newClass = Class.forName("BadMapIstore"); + throw new RuntimeException( + "StackMapCheck failed, BadMapIstore did not throw VerifyError"); + } catch (java.lang.VerifyError e) { + System.out.println("BadMapIstore passed, VerifyError was thrown"); + } + } +} -- GitLab