From cadbb7fae0e2de56e11845edecce663bf184641f Mon Sep 17 00:00:00 2001 From: jfranck Date: Fri, 22 Nov 2013 11:34:26 +0100 Subject: [PATCH] 8023278: Reflection API methods do not throw AnnotationFormatError in case of malformed Runtime[In]VisibleTypeAnnotations attribute Reviewed-by: darcy --- .../reflect/annotation/TypeAnnotation.java | 20 ++-- .../annotation/TypeAnnotationParser.java | 31 ++--- .../typeAnnotations/BadCPIndex.java | 106 ++++++++++++++++++ 3 files changed, 134 insertions(+), 23 deletions(-) create mode 100644 test/java/lang/annotation/typeAnnotations/BadCPIndex.java diff --git a/src/share/classes/sun/reflect/annotation/TypeAnnotation.java b/src/share/classes/sun/reflect/annotation/TypeAnnotation.java index 5c94db4d6..e1d1bba5e 100644 --- a/src/share/classes/sun/reflect/annotation/TypeAnnotation.java +++ b/src/share/classes/sun/reflect/annotation/TypeAnnotation.java @@ -147,13 +147,13 @@ public final class TypeAnnotation { public static final LocationInfo BASE_LOCATION = new LocationInfo(); public static LocationInfo parseLocationInfo(ByteBuffer buf) { - int depth = buf.get(); + int depth = buf.get() & 0xFF; if (depth == 0) return BASE_LOCATION; Location[] locations = new Location[depth]; for (int i = 0; i < depth; i++) { byte tag = buf.get(); - byte index = buf.get(); + short index = (short)(buf.get() & 0xFF); if (!(tag == 0 || tag == 1 | tag == 2 || tag == 3)) throw new AnnotationFormatError("Bad Location encoding in Type Annotation"); if (tag != 3 && index != 0) @@ -164,26 +164,26 @@ public final class TypeAnnotation { } public LocationInfo pushArray() { - return pushLocation((byte)0, (byte)0); + return pushLocation((byte)0, (short)0); } public LocationInfo pushInner() { - return pushLocation((byte)1, (byte)0); + return pushLocation((byte)1, (short)0); } public LocationInfo pushWildcard() { - return pushLocation((byte) 2, (byte) 0); + return pushLocation((byte) 2, (short) 0); } - public LocationInfo pushTypeArg(byte index) { + public LocationInfo pushTypeArg(short index) { return pushLocation((byte) 3, index); } - public LocationInfo pushLocation(byte tag, byte index) { + public LocationInfo pushLocation(byte tag, short index) { int newDepth = this.depth + 1; Location[] res = new Location[newDepth]; System.arraycopy(this.locations, 0, res, 0, depth); - res[newDepth - 1] = new Location(tag, index); + res[newDepth - 1] = new Location(tag, (short)(index & 0xFF)); return new LocationInfo(newDepth, res); } @@ -207,13 +207,13 @@ public final class TypeAnnotation { public static final class Location { public final byte tag; - public final byte index; + public final short index; boolean isSameLocation(Location other) { return tag == other.tag && index == other.index; } - public Location(byte tag, byte index) { + public Location(byte tag, short index) { this.tag = tag; this.index = index; } diff --git a/src/share/classes/sun/reflect/annotation/TypeAnnotationParser.java b/src/share/classes/sun/reflect/annotation/TypeAnnotationParser.java index abd28cfaf..9bd4b6c64 100644 --- a/src/share/classes/sun/reflect/annotation/TypeAnnotationParser.java +++ b/src/share/classes/sun/reflect/annotation/TypeAnnotationParser.java @@ -394,20 +394,25 @@ public final class TypeAnnotationParser { ConstantPool cp, AnnotatedElement baseDecl, Class container) { - TypeAnnotationTargetInfo ti = parseTargetInfo(buf); - LocationInfo locationInfo = LocationInfo.parseLocationInfo(buf); - Annotation a = AnnotationParser.parseAnnotation(buf, cp, container, false); - if (ti == null) // Inside a method for example - return null; - return new TypeAnnotation(ti, locationInfo, a, baseDecl); + try { + TypeAnnotationTargetInfo ti = parseTargetInfo(buf); + LocationInfo locationInfo = LocationInfo.parseLocationInfo(buf); + Annotation a = AnnotationParser.parseAnnotation(buf, cp, container, false); + if (ti == null) // Inside a method for example + return null; + return new TypeAnnotation(ti, locationInfo, a, baseDecl); + } catch (IllegalArgumentException | // Bad type in const pool at specified index + BufferUnderflowException e) { + throw new AnnotationFormatError(e); + } } private static TypeAnnotationTargetInfo parseTargetInfo(ByteBuffer buf) { - byte posCode = buf.get(); + int posCode = buf.get() & 0xFF; switch(posCode) { case CLASS_TYPE_PARAMETER: case METHOD_TYPE_PARAMETER: { - byte index = buf.get(); + int index = buf.get() & 0xFF; TypeAnnotationTargetInfo res; if (posCode == CLASS_TYPE_PARAMETER) res = new TypeAnnotationTargetInfo(TypeAnnotationTarget.CLASS_TYPE_PARAMETER, @@ -418,7 +423,7 @@ public final class TypeAnnotationParser { return res; } // unreachable break; case CLASS_EXTENDS: { - short index = buf.getShort(); + short index = buf.getShort(); //needs to be signed if (index == -1) { return new TypeAnnotationTargetInfo(TypeAnnotationTarget.CLASS_EXTENDS); } else if (index >= 0) { @@ -437,7 +442,7 @@ public final class TypeAnnotationParser { case METHOD_RECEIVER: return new TypeAnnotationTargetInfo(TypeAnnotationTarget.METHOD_RECEIVER); case METHOD_FORMAL_PARAMETER: { - byte index = buf.get(); + int index = buf.get() & 0xFF; return new TypeAnnotationTargetInfo(TypeAnnotationTarget.METHOD_FORMAL_PARAMETER, index); } //unreachable break; @@ -486,12 +491,12 @@ public final class TypeAnnotationParser { } private static TypeAnnotationTargetInfo parseShortTarget(TypeAnnotationTarget target, ByteBuffer buf) { - short index = buf.getShort(); + int index = buf.getShort() & 0xFFFF; return new TypeAnnotationTargetInfo(target, index); } private static TypeAnnotationTargetInfo parse2ByteTarget(TypeAnnotationTarget target, ByteBuffer buf) { - byte count = buf.get(); - byte secondaryIndex = buf.get(); + int count = buf.get() & 0xFF; + int secondaryIndex = buf.get() & 0xFF; return new TypeAnnotationTargetInfo(target, count, secondaryIndex); diff --git a/test/java/lang/annotation/typeAnnotations/BadCPIndex.java b/test/java/lang/annotation/typeAnnotations/BadCPIndex.java new file mode 100644 index 000000000..9985f23ab --- /dev/null +++ b/test/java/lang/annotation/typeAnnotations/BadCPIndex.java @@ -0,0 +1,106 @@ +/* + * Copyright (c) 2013, 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 8023878 + * @summary Test that the right kind of exception is thrown from the type + * annotation reflection code. + * @run testng BadCPIndex + */ + +import java.lang.annotation.*; +import java.util.Base64; +import java.util.function.Function; + +import org.testng.annotations.Test; +import org.testng.annotations.DataProvider; + +public class BadCPIndex { + private static final MyLoader loader = new MyLoader(BadCPIndex.class.getClassLoader()); + + // Blueprint for broken C + //public static class C extends @BadCPIndex.A Object {} + private static final String encodedBrokenC = "yv66vgAAADQAFgoAAwAPBwARBwATAQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEAClNvdXJjZUZpbGUBAA9CYWRDUEluZGV4LmphdmEBAB1SdW50aW1lVmlzaWJsZVR5cGVBbm5vdGF0aW9ucwcAFAEAAUEBAAxJbm5lckNsYXNzZXMBAA5MQmFkQ1BJbmRleCRBOwwABAAFBwAVAQAMQmFkQ1BJbmRleCRDAQABQwEAEGphdmEvbGFuZy9PYmplY3QBAAxCYWRDUEluZGV4JEEBAApCYWRDUEluZGV4ACEAAgADAAAAAAABAAEABAAFAAEABgAAAB0AAQABAAAABSq3AAGxAAAAAQAHAAAABgABAAAAKQADAAgAAAACAAkACgAAAAoAARD//wAADwAAAA0AAAASAAIACwAQAAwmCQACABAAEgAJ"; + + // Blueprint for broken D + //public static class D<@BadCPIndex.B U> {} + private static final String encodedBrokenD = "yv66vgAAADQAGAoAAwARBwATBwAVAQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEACVNpZ25hdHVyZQEAKDxVOkxqYXZhL2xhbmcvT2JqZWN0Oz5MamF2YS9sYW5nL09iamVjdDsBAApTb3VyY2VGaWxlAQAPQmFkQ1BJbmRleC5qYXZhAQAdUnVudGltZVZpc2libGVUeXBlQW5ub3RhdGlvbnMHABYBAAFCAQAMSW5uZXJDbGFzc2VzAQAOTEJhZENQSW5kZXgkQjsMAAQABQcAFwEADEJhZENQSW5kZXgkRAEAAUQBABBqYXZhL2xhbmcvT2JqZWN0AQAMQmFkQ1BJbmRleCRCAQAKQmFkQ1BJbmRleAAhAAIAAwAAAAAAAQABAAQABQABAAYAAAAdAAEAAQAAAAUqtwABsQAAAAEABwAAAAYAAQAAAEAABAAIAAAAAgAJAAoAAAACAAsADAAAAAkAAQAAAAARAAAADwAAABIAAgANABIADiYJAAIAEgAUAAk="; + + // Blueprint for broken E + //public static class E extends @BadCPIndex.A Object {} + private static final String encodedBrokenE = "yv66vgAAADQAFgoAAwAPBwARBwATAQAGPGluaXQ+AQADKClWAQAEQ29kZQEAD0xpbmVOdW1iZXJUYWJsZQEAClNvdXJjZUZpbGUBAA9CYWRDUEluZGV4LmphdmEBAB1SdW50aW1lVmlzaWJsZVR5cGVBbm5vdGF0aW9ucwcAFAEAAUEBAAxJbm5lckNsYXNzZXMBAA5MQmFkQ1BJbmRleCRBOwwABAAFBwAVAQAMQmFkQ1BJbmRleCRFAQABRQEAEGphdmEvbGFuZy9PYmplY3QBAAxCYWRDUEluZGV4JEEBAApCYWRDUEluZGV4ACEAAgADAAAAAAABAAEABAAFAAEABgAAAB0AAQABAAAABSq3AAGxAAAAAQAHAAAABgABAAAARgADAAgAAAACAAkACgAAAAoAARD//wAADgAKAA0AAAASAAIACwAQAAwmCQACABAAEgAJ"; + + private static final Object[][] cases = { + { new Case("BadCPIndex$C", encodedBrokenC, Class::getAnnotatedSuperclass) }, + { new Case("BadCPIndex$D", encodedBrokenD, (c -> c.getTypeParameters()[0].getAnnotations()))}, + { new Case("BadCPIndex$E", encodedBrokenE, Class::getAnnotatedSuperclass) }, + }; + + @DataProvider + public static Object[][] data() { return cases; } + + @Test(dataProvider="data") + public static void testOpThrowsAFE(Case testCase) { + Class c = loader.defineClass(testCase.name, Base64.getDecoder().decode(testCase.encoding)); + try { + System.out.println("Testing: " + c); + testCase.trigger.apply(c); + throw new RuntimeException("Expecting AnnotationFormatError here"); + } catch (AnnotationFormatError e) { + ; //ok + } catch (Exception e) { + e.printStackTrace(); + } + } + + private static class MyLoader extends ClassLoader { + public MyLoader(ClassLoader parent) { + super(parent); + } + + public Class defineClass(String name, byte[] bytes) { + return defineClass(name, bytes, 0, bytes.length); + } + } + + private static class Case { + public String name; + public String encoding; + public Function, Object> trigger; + + public Case(String name, String encoding, Function, Object> trigger) { + this.name = name; + this.encoding = encoding; + this.trigger = trigger; + } + } + + @Target(ElementType.TYPE_USE) + @Retention(RetentionPolicy.RUNTIME) + public static @interface A {} + + @Target(ElementType.TYPE_PARAMETER) + @Retention(RetentionPolicy.RUNTIME) + public static @interface B {} +} -- GitLab