From 8721704efe1e0255a7113d7e3637093f79f47299 Mon Sep 17 00:00:00 2001 From: Shivam7-1 <55046031+Shivam7-1@users.noreply.github.com> Date: Tue, 28 Jan 2025 09:20:51 +0530 Subject: [PATCH 1/2] Add validation and exception handling in readAttribute --- .../org/apache/bcel/classfile/Attribute.java | 48 ++++++++++++------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/main/java/org/apache/bcel/classfile/Attribute.java b/src/main/java/org/apache/bcel/classfile/Attribute.java index f2b2e4e71a..7a55c2644d 100644 --- a/src/main/java/org/apache/bcel/classfile/Attribute.java +++ b/src/main/java/org/apache/bcel/classfile/Attribute.java @@ -18,6 +18,7 @@ */ package org.apache.bcel.classfile; +import java.security.InvalidParameterException; import java.io.DataInput; import java.io.DataInputStream; import java.io.DataOutputStream; @@ -108,25 +109,36 @@ protected static void println(final String msg) { * @throws IOException if an I/O error occurs. * @since 6.0 */ - public static Attribute readAttribute(final DataInput dataInput, final ConstantPool constantPool) throws IOException { - byte tag = Const.ATTR_UNKNOWN; // Unknown attribute - // Get class name from constant pool via 'name_index' indirection - final int nameIndex = dataInput.readUnsignedShort(); - final String name = constantPool.getConstantUtf8(nameIndex).getBytes(); - - // Length of data in bytes - final int length = dataInput.readInt(); - - // Compare strings to find known attribute - for (byte i = 0; i < Const.KNOWN_ATTRIBUTES; i++) { - if (name.equals(Const.getAttributeName(i))) { - tag = i; // found! - break; - } +public static Attribute readAttribute(final DataInput dataInput, final ConstantPool constantPool) throws IOException { + byte tag = Const.ATTR_UNKNOWN; // Unknown attribute + + // Get class name from constant pool via 'name_index' indirection + final int nameIndex = dataInput.readUnsignedShort(); + final String name = constantPool.getConstantUtf8(nameIndex).getBytes(); + + // Validate name + if (name == null || name.isEmpty()) { + throw new InvalidParameterException("Attribute name is invalid or empty"); + } + + // Length of data in bytes + final int length = dataInput.readInt(); + + // Validate length + if (length < 0) { + throw new InvalidParameterException("Attribute length is negative"); + } + + // Compare strings to find known attribute + for (byte i = 0; i < Const.KNOWN_ATTRIBUTES; i++) { + if (name.equals(Const.getAttributeName(i))) { + tag = i; // found! + break; } + } - // Call proper constructor, depending on 'tag' - switch (tag) { + // Call proper constructor, depending on 'tag' + switch (tag) { case Const.ATTR_UNKNOWN: final Object r = READERS.get(name); if (r instanceof UnknownAttributeReader) { @@ -197,8 +209,8 @@ public static Attribute readAttribute(final DataInput dataInput, final ConstantP default: // Never reached throw new IllegalStateException("Unrecognized attribute type tag parsed: " + tag); - } } +} /** * Class method reads one attribute from the input data stream. This method must not be accessible from the outside. It From 9c934206ed22134cd8041ea09be17e4c8a959474 Mon Sep 17 00:00:00 2001 From: Shivam7-1 <55046031+Shivam7-1@users.noreply.github.com> Date: Sat, 1 Feb 2025 15:22:23 +0000 Subject: [PATCH 2/2] Added unit test and fixed indentation issues --- .../org/apache/bcel/classfile/Attribute.java | 47 ++++++++-------- .../apache/bcel/data/readAttributeTest.java | 54 +++++++++++++++++++ 2 files changed, 75 insertions(+), 26 deletions(-) create mode 100644 src/test/java/org/apache/bcel/data/readAttributeTest.java diff --git a/src/main/java/org/apache/bcel/classfile/Attribute.java b/src/main/java/org/apache/bcel/classfile/Attribute.java index 7a55c2644d..65579baa42 100644 --- a/src/main/java/org/apache/bcel/classfile/Attribute.java +++ b/src/main/java/org/apache/bcel/classfile/Attribute.java @@ -109,33 +109,28 @@ protected static void println(final String msg) { * @throws IOException if an I/O error occurs. * @since 6.0 */ -public static Attribute readAttribute(final DataInput dataInput, final ConstantPool constantPool) throws IOException { - byte tag = Const.ATTR_UNKNOWN; // Unknown attribute - - // Get class name from constant pool via 'name_index' indirection - final int nameIndex = dataInput.readUnsignedShort(); - final String name = constantPool.getConstantUtf8(nameIndex).getBytes(); - - // Validate name - if (name == null || name.isEmpty()) { - throw new InvalidParameterException("Attribute name is invalid or empty"); - } - - // Length of data in bytes - final int length = dataInput.readInt(); - - // Validate length - if (length < 0) { - throw new InvalidParameterException("Attribute length is negative"); - } - - // Compare strings to find known attribute - for (byte i = 0; i < Const.KNOWN_ATTRIBUTES; i++) { - if (name.equals(Const.getAttributeName(i))) { - tag = i; // found! - break; + public static Attribute readAttribute(final DataInput dataInput, final ConstantPool constantPool) throws IOException { + byte tag = Const.ATTR_UNKNOWN; // Unknown attribute + // Get class name from constant pool via 'name_index' indirection + final int nameIndex = dataInput.readUnsignedShort(); + final String name = constantPool.getConstantUtf8(nameIndex).getBytes(); + // Validate name + if (name == null || name.isEmpty()) { + throw new InvalidParameterException("Attribute name is invalid or empty"); + } + // Length of data in bytes + final int length = dataInput.readInt(); + // Validate length + if (length < 0) { + throw new InvalidParameterException("Attribute length is negative"); + } + // Compare strings to find known attribute + for (byte i = 0; i < Const.KNOWN_ATTRIBUTES; i++) { + if (name.equals(Const.getAttributeName(i))) { + tag = i; // found! + break; + } } - } // Call proper constructor, depending on 'tag' switch (tag) { diff --git a/src/test/java/org/apache/bcel/data/readAttributeTest.java b/src/test/java/org/apache/bcel/data/readAttributeTest.java new file mode 100644 index 0000000000..9a3bddf366 --- /dev/null +++ b/src/test/java/org/apache/bcel/data/readAttributeTest.java @@ -0,0 +1,54 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + + package org.apache.bcel.data; + + import static org.junit.jupiter.api.Assertions.assertThrows; + import java.io.DataInputStream; + import java.io.ByteArrayInputStream; + import java.security.InvalidParameterException; + import org.apache.bcel.classfile.Attribute; + import org.apache.bcel.classfile.Constant; + import org.apache.bcel.classfile.ConstantPool; + import org.junit.jupiter.api.Test; + + public class readAttributeTest { + + @Test + public void testReadAttributeWithInvalidName() { + DataInputStream dataInput = new DataInputStream(new ByteArrayInputStream(new byte[]{0, 0, 0, 0, 0})); + Constant[] constants = new Constant[1]; + ConstantPool constantPool = new ConstantPool(constants); + + assertThrows(InvalidParameterException.class, () -> { + Attribute.readAttribute(dataInput, constantPool); + }); + } + + @Test + public void testReadAttributeWithNegativeLength() { + DataInputStream dataInput = new DataInputStream(new ByteArrayInputStream(new byte[]{0, 0, 0, 0, -1})); + Constant[] constants = new Constant[1]; + ConstantPool constantPool = new ConstantPool(constants); + + assertThrows(InvalidParameterException.class, () -> { + Attribute.readAttribute(dataInput, constantPool); + }); + } + } \ No newline at end of file