reject location aliased to two object types in reflection verify

rust/reflection/src/lib.rs

@@ -56,6 +56,8 @@ pub enum FlatbufferError {
     InvalidUnionEnum,
     #[error("Table or Struct doesn't belong to the buffer")]
     InvalidTableOrStruct,
+    #[error("Buffer location {0} is reachable as two different object types")]
+    ConflictingObjectType(usize),
     #[error("Field not found in the table schema")]
     FieldNotFound,
 }

rust/reflection/src/reflection_verifier.rs

@@ -48,6 +48,28 @@ pub fn verify_with_options(
     Err(FlatbufferError::InvalidSchema)
 }
 
+// Records that `buf_loc` is reached as object `obj_idx`. The accessor
+// (`SafeBuffer::get_all_fields`) resolves a location to a single object type
+// through this map, and the table dedup below skips a location once it is
+// verified. A location claimed by two different object types would let a later
+// read reinterpret bytes that were only ever verified under the other type, so
+// reject that instead. Forward-only uoffsets keep the root out of this map's
+// conflict range, so same-type sharing (the only legitimate aliasing) still
+// passes.
+fn record_obj_idx(
+    buf_loc_to_obj_idx: &mut HashMap<usize, i32>,
+    buf_loc: usize,
+    obj_idx: i32,
+) -> FlatbufferResult<()> {
+    if let Some(&existing) = buf_loc_to_obj_idx.get(&buf_loc) {
+        if existing != obj_idx {
+            return Err(FlatbufferError::ConflictingObjectType(buf_loc));
+        }
+    }
+    buf_loc_to_obj_idx.insert(buf_loc, obj_idx);
+    Ok(())
+}
+
 fn verify_table(
     verifier: &mut Verifier,
     table_object: &Object,
@@ -111,7 +133,7 @@ fn verify_table(
                     let object_index = field.type_().index();
                     let child_obj = schema.objects().get(object_index.try_into()?);
                     if child_obj.is_struct() {
-                        buf_loc_to_obj_idx.insert(field_pos, object_index);
+                        record_obj_idx(buf_loc_to_obj_idx, field_pos, object_index)?;
                         verify_struct(
                             table_verifier.verifier(),
                             &child_obj,
@@ -122,7 +144,7 @@ fn verify_table(
                     } else {
                         let field_value = table_verifier.verifier().get_uoffset(field_pos)?;
                         let table_pos = field_pos.saturating_add(field_value.try_into()?);
-                        buf_loc_to_obj_idx.insert(table_pos, object_index);
+                        record_obj_idx(buf_loc_to_obj_idx, table_pos, object_index)?;
                         verify_table(
                             table_verifier.verifier(),
                             &child_obj,
@@ -181,7 +203,7 @@ fn verify_struct(
             let child_obj = schema.objects().get(obj_idx.try_into()?);
             if child_obj.is_struct() {
                 let field_pos = struct_pos.saturating_add(field.offset().into());
-                buf_loc_to_obj_idx.insert(field_pos, obj_idx);
+                record_obj_idx(buf_loc_to_obj_idx, field_pos, obj_idx)?;
                 verify_struct(verifier, &child_obj, field_pos, schema, buf_loc_to_obj_idx)?;
             }
         }
@@ -299,7 +321,7 @@ fn verify_vector<'a, 'b, 'c>(
                         end: vector_start.saturating_add(vector_size.try_into()?),
                     };
                     for struct_pos in vector_range.step_by(child_obj.bytesize().try_into()?) {
-                        buf_loc_to_obj_idx.insert(struct_pos, child_obj_idx);
+                        record_obj_idx(buf_loc_to_obj_idx, struct_pos, child_obj_idx)?;
                         verify_struct(
                             verifier,
                             &child_obj,
@@ -319,7 +341,7 @@ fn verify_vector<'a, 'b, 'c>(
                     for element_pos in vector_range.step_by(SIZE_UOFFSET) {
                         let table_pos = element_pos
                             .saturating_add(verifier.get_uoffset(element_pos)?.try_into()?);
-                        buf_loc_to_obj_idx.insert(table_pos, child_obj_idx);
+                        record_obj_idx(buf_loc_to_obj_idx, table_pos, child_obj_idx)?;
                         verify_table(
                             verifier,
                             &child_obj,
@@ -369,7 +391,7 @@ fn verify_union<'a, 'b, 'c>(
             BaseType::String => <&str>::run_verifier(table_verifier.verifier(), union_pos)?,
             BaseType::Obj => {
                 let child_obj = schema.objects().get(enum_type.index().try_into()?);
-                buf_loc_to_obj_idx.insert(union_pos, enum_type.index());
+                record_obj_idx(buf_loc_to_obj_idx, union_pos, enum_type.index())?;
                 if child_obj.is_struct() {
                     verify_struct(
                         table_verifier.verifier(),

tests/rust_reflection_test/src/lib.rs

@@ -1399,6 +1399,24 @@ fn test_create_safe_buffer_limit_max_size_fails() {
     assert!(format!("{:#?}", safe_buffer.err().unwrap()).contains("ApparentSizeTooLarge"));
 }
 
+#[test]
+fn test_create_safe_buffer_aliased_field_types_fails() {
+    // `enemy` (Monster) and `testempty` (Stat) are rewritten to point at the
+    // same child table. Before the fix the verifier deduped on position alone,
+    // so the Stat interpretation was never checked and a later read through the
+    // Stat layout reinterpreted unverified Monster bytes.
+    let (buffer, table_pos) = create_aliased_field_buffer();
+    let schema_buffer = load_file_as_buffer("../monster_test.bfbs");
+    let schema = root_as_schema(schema_buffer.as_slice()).unwrap();
+
+    let safe_buffer = SafeBuffer::new(&buffer, &schema);
+
+    assert_eq!(
+        safe_buffer.err().unwrap(),
+        FlatbufferError::ConflictingObjectType(table_pos)
+    );
+}
+
 #[test]
 fn test_safe_buffer_integer_same_type_succeeds() {
     let buffer = create_test_buffer();
@@ -2041,3 +2059,51 @@ fn create_test_buffer() -> Vec<u8> {
     let (flatbuf, start_loc) = builder.mut_finished_buffer();
     flatbuf[start_loc..].to_vec()
 }
+
+// Builds a monster with both `enemy` (Monster) and `testempty` (Stat) set, then
+// rewrites the root vtable so both fields resolve to the same child table
+// position. Returns the tampered buffer and that shared table position.
+fn create_aliased_field_buffer() -> (Vec<u8>, usize) {
+    use std::convert::TryInto;
+    let mut builder = FlatBufferBuilder::new();
+    let enemy_name = builder.create_string("EnemyMonster");
+    let enemy = my_game::example::Monster::create(
+        &mut builder,
+        &my_game::example::MonsterArgs { name: Some(enemy_name), ..Default::default() },
+    );
+    let fred_name = builder.create_string("Fred");
+    let stat = my_game::example::Stat::create(
+        &mut builder,
+        &my_game::example::StatArgs { id: Some(fred_name), ..Default::default() },
+    );
+    let name = builder.create_string("MyMonster");
+    let mon = my_game::example::Monster::create(
+        &mut builder,
+        &my_game::example::MonsterArgs {
+            name: Some(name),
+            enemy: Some(enemy),
+            testempty: Some(stat),
+            ..Default::default()
+        },
+    );
+    my_game::example::finish_monster_buffer(&mut builder, mon);
+
+    let schema_buffer = load_file_as_buffer("../monster_test.bfbs");
+    let enemy_voff = get_schema_field(&schema_buffer, "enemy").offset() as usize;
+    let testempty_voff = get_schema_field(&schema_buffer, "testempty").offset() as usize;
+
+    let mut buf = builder.finished_data().to_vec();
+    let rd_u32 = |b: &[u8], i: usize| u32::from_le_bytes(b[i..i + 4].try_into().unwrap());
+    let rd_i32 = |b: &[u8], i: usize| i32::from_le_bytes(b[i..i + 4].try_into().unwrap());
+    let rd_u16 = |b: &[u8], i: usize| u16::from_le_bytes(b[i..i + 2].try_into().unwrap());
+
+    let table_loc = rd_u32(&buf, 0) as usize;
+    let vtable_loc = (table_loc as i32 - rd_i32(&buf, table_loc)) as usize;
+    let enemy_slot = rd_u16(&buf, vtable_loc + enemy_voff);
+    buf[vtable_loc + testempty_voff..vtable_loc + testempty_voff + 2]
+        .copy_from_slice(&enemy_slot.to_le_bytes());
+
+    let field_pos = table_loc + enemy_slot as usize;
+    let table_pos = field_pos + rd_u32(&buf, field_pos) as usize;
+    (buf, table_pos)
+}