Skip to content

Commit d361c9e

Browse files
committed
Address classfile review findings
1 parent 8a2e456 commit d361c9e

6 files changed

Lines changed: 32 additions & 6 deletions

File tree

classfile/src/opcode.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,8 @@ impl Opcode {
406406
0x69 => success(Opcode::Lmul).parse(data),
407407
0x75 => success(Opcode::Lneg).parse(data),
408408
0xab => flat_map((take((4 - (offset + 1) % 4) % 4), be_i32, be_i32), |(_, default, npairs)| {
409-
move |x| {
410-
if npairs < 0 {
409+
move |x: &'a [u8]| {
410+
if npairs < 0 || npairs as usize > x.len() / 8 {
411411
return Err(nom::Err::Error(Error::new(x, ErrorKind::Verify)));
412412
}
413413
map(count((be_i32, be_i32), npairs as usize), |offsets| Opcode::Lookupswitch(default, offsets)).parse(x)
@@ -469,11 +469,11 @@ impl Opcode {
469469
0x11 => map(be_i16, Opcode::Sipush).parse(data),
470470
0x5f => success(Opcode::Swap).parse(data),
471471
0xaa => flat_map((take((4 - (offset + 1) % 4) % 4), be_i32, be_i32, be_i32), |(_, default, low, high)| {
472-
move |x| {
472+
move |x: &'a [u8]| {
473473
let Some(entry_count) = high.checked_sub(low).and_then(|range| range.checked_add(1)) else {
474474
return Err(nom::Err::Error(Error::new(x, ErrorKind::Verify)));
475475
};
476-
if entry_count < 0 {
476+
if entry_count <= 0 || entry_count as usize > x.len() / 4 {
477477
return Err(nom::Err::Error(Error::new(x, ErrorKind::Verify)));
478478
}
479479
map(count(be_i32, entry_count as usize), |offsets| {

classfile/tests/test.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
use std::collections::BTreeMap;
2+
13
use java_constants::ClassAccessFlags;
24

35
use classfile::{AttributeInfo, ClassFileError, ClassInfo, ConstantPoolReference, Opcode};
@@ -114,6 +116,16 @@ fn test_switch() {
114116
}
115117
}
116118

119+
#[test]
120+
fn test_switch_rejects_entry_counts_larger_than_remaining_input() {
121+
let constant_pool = BTreeMap::new();
122+
let lookup_switch = [0xab, 0, 0, 0, 0, 0, 0, 0, 0x7f, 0xff, 0xff, 0xff];
123+
assert!(Opcode::parse(&lookup_switch, 0, &constant_pool).is_err());
124+
125+
let table_switch = [0xaa, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0x7f, 0xff, 0xff, 0xff];
126+
assert!(Opcode::parse(&table_switch, 0, &constant_pool).is_err());
127+
}
128+
117129
#[test]
118130
fn test_invokeinterface() {
119131
let interface = include_bytes!("../../test_data/Interface.class");

jvm_rust/src/class_definition.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -164,15 +164,22 @@ impl ClassDefinitionImpl {
164164
}
165165
Opcode::Invokeinterface(ConstantPoolReference::InterfaceMethodref(reference), _, _)
166166
| Opcode::Invokespecial(ConstantPoolReference::Method(reference))
167-
| Opcode::Invokestatic(ConstantPoolReference::Method(reference))
168-
| Opcode::Invokevirtual(ConstantPoolReference::Method(reference)) => {
167+
| Opcode::Invokestatic(ConstantPoolReference::Method(reference)) => {
169168
if reference.class.is_empty()
170169
|| reference.class.starts_with('[')
171170
|| !matches!(JavaType::try_parse(&reference.descriptor), Some(JavaType::Method(_, _)))
172171
{
173172
return Err(ClassFileError::InvalidFormat);
174173
}
175174
}
175+
Opcode::Invokevirtual(ConstantPoolReference::Method(reference)) => {
176+
if reference.class.is_empty()
177+
|| (reference.class.starts_with('[') && !matches!(JavaType::try_parse(&reference.class), Some(JavaType::Array(_))))
178+
|| !matches!(JavaType::try_parse(&reference.descriptor), Some(JavaType::Method(_, _)))
179+
{
180+
return Err(ClassFileError::InvalidFormat);
181+
}
182+
}
176183
Opcode::Anewarray(ConstantPoolReference::Class(name))
177184
| Opcode::Checkcast(ConstantPoolReference::Class(name))
178185
| Opcode::Instanceof(ConstantPoolReference::Class(name))

test_data/Array.class

104 Bytes
Binary file not shown.

test_data/Array.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,5 @@ test한글
1212
10
1313
10
1414
10
15+
112344
16+
654321

test_data/src/Array.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,5 +31,10 @@ public static void main(String[] args) {
3131
System.out.println(char_array.length);
3232
System.out.println(boolean_array.length);
3333
System.out.println(string_array.length);
34+
35+
int[] cloned_int_array = (int[]) int_array.clone();
36+
cloned_int_array[0] = 654321;
37+
System.out.println(int_array[0]);
38+
System.out.println(cloned_int_array[0]);
3439
}
3540
}

0 commit comments

Comments
 (0)