validate internal references on parse

src/data/displacement.rs

@@ -51,6 +51,12 @@ pub struct DisplacementNeighbour {
     pub sub_neighbours: [Option<DisplacementSubNeighbour>; 2],
 }
 
+impl DisplacementNeighbour {
+    pub fn iter(&self) -> impl Iterator<Item = &DisplacementSubNeighbour> {
+        self.sub_neighbours.iter().filter_map(Option::as_ref)
+    }
+}
+
 impl BinRead for DisplacementNeighbour {
     type Args = ();
 
@@ -133,9 +139,18 @@ pub enum NeighbourOrientation {
 
 #[derive(Debug, Clone, BinRead)]
 pub struct DisplacementCornerNeighbour {
-    pub neighbours: [u16; 4],
+    neighbours: [u16; 4],
     #[br(align_after = align_of::< DisplacementCornerNeighbour > ())]
-    pub neighbour_count: u8,
+    neighbour_count: u8,
+}
+
+impl DisplacementCornerNeighbour {
+    pub fn neighbours(&self) -> impl Iterator<Item = u16> + '_ {
+        self.neighbours
+            .iter()
+            .copied()
+            .take(self.neighbour_count as usize)
+    }
 }
 
 static_assertions::const_assert_eq!(size_of::<DisplacementCornerNeighbour>(), 10);

src/data/mod.rs

@@ -405,8 +405,8 @@ pub struct SurfaceEdge {
 }
 
 impl SurfaceEdge {
-    pub fn edge_index(&self) -> usize {
+    pub fn edge_index(&self) -> u32 {
-        self.edge.abs() as usize
+        self.edge.abs() as u32
     }
 
     pub fn direction(&self) -> EdgeDirection {
@@ -439,6 +439,12 @@ pub struct Face {
     pub smoothing_groups: u32,
 }
 
+impl Face {
+    pub fn displacement_index(&self) -> Option<i16> {
+        (self.displacement_info >= 0).then(|| self.displacement_info)
+    }
+}
+
 static_assertions::const_assert_eq!(size_of::<Face>(), 56);
 
 #[derive(Default, Debug, Clone)]

src/error.rs

@@ -22,6 +22,8 @@ pub enum BspError {
     String(#[from] StringError),
     #[error("Malformed field found while parsing: {0:#}")]
     MalformedData(binrw::Error),
+    #[error("bsp file is well-formed but contains invalid data")]
+    Validation(#[from] ValidationError),
 }
 
 impl From<binrw::Error> for BspError {
@@ -61,3 +63,16 @@ pub enum StringError {
     #[error("String is not null-terminated")]
     NotNullTerminated,
 }
+
+#[derive(Debug, Error)]
+pub enum ValidationError {
+    #[error(
+        "A {source_} indexes into {target} but the index {index} is out of range of the size {size}"
+    )]
+    ReferenceOutOfRange {
+        source_: &'static str,
+        target: &'static str,
+        index: i64,
+        size: usize,
+    },
+}

src/handle/displacement.rs

@@ -16,8 +16,7 @@ impl<'a> Handle<'a, DisplacementInfo> {
         self.data
             .corner_neighbours
             .iter()
-            .flat_map(|corner| &corner.neighbours[0..corner.neighbour_count.min(4) as usize])
+            .flat_map(|corner| corner.neighbours())
-            .copied()
             .filter_map(|id| self.bsp.displacement(id as usize))
     }
 

src/handle/face.rs

@@ -29,7 +29,7 @@ impl<'a> Handle<'a, Face> {
             .flat_map(move |surface_edge| bsp.surface_edges.get(surface_edge as usize))
             .flat_map(move |surface_edge| {
                 bsp.edges
-                    .get(surface_edge.edge_index())
+                    .get(surface_edge.edge_index() as usize)
                     .map(|edge| (edge, surface_edge.direction()))
             })
             .map(|(edge, direction)| match direction {

src/lib.rs

@@ -8,6 +8,7 @@ use crate::bspfile::LumpType;
 pub use crate::data::TextureFlags;
 pub use crate::data::Vector;
 use crate::data::*;
+use crate::error::ValidationError;
 pub use crate::handle::Handle;
 use binrw::io::Cursor;
 use binrw::BinRead;
@@ -105,7 +106,7 @@ fn test_leaf_clusters() {
         .clusters()
         .map(|cluster| cluster.map(|leaf| leaf.contents).collect())
         .collect();
-    assert_eq!(vec![vec![0, 1], vec![2], vec![3, 4],], clustered);
+    assert_eq!(vec![vec![0, 1], vec![2], vec![3, 4]], clustered);
 }
 
 impl From<Vec<Leaf>> for Leaves {
@@ -238,31 +239,31 @@ impl Bsp {
             .lump_reader(LumpType::DisplacementTris)?
             .read_vec(|r| r.read())?;
 
-        Ok({
+        let bsp = Bsp {
-            Bsp {
+            header: bsp_file.header().clone(),
-                header: bsp_file.header().clone(),
+            entities,
-                entities,
+            textures_data,
-                textures_data,
+            textures_info,
-                textures_info,
+            planes,
-                planes,
+            nodes,
-                nodes,
+            leaves,
-                leaves,
+            leaf_faces,
-                leaf_faces,
+            leaf_brushes,
-                leaf_brushes,
+            models,
-                models,
+            brushes,
-                brushes,
+            brush_sides,
-                brush_sides,
+            vertices,
-                vertices,
+            edges,
-                edges,
+            surface_edges,
-                surface_edges,
+            faces,
-                faces,
+            original_faces,
-                original_faces,
+            vis_data,
-                vis_data,
+            displacements,
-                displacements,
+            displacement_vertices,
-                displacement_vertices,
+            displacement_triangles,
-                displacement_triangles,
+        };
-            }
+        bsp.validate()?;
-        })
+        Ok(bsp)
     }
 
     pub fn leaf(&self, n: usize) -> Option<Handle<'_, Leaf>> {
@@ -287,18 +288,12 @@ impl Bsp {
             .map(|displacement| Handle::new(self, displacement))
     }
 
-    pub fn displacement_vertex(&self, n: usize) -> Option<Handle<'_, DisplacementVertex>> {
+    fn displacement_vertex(&self, n: usize) -> Option<Handle<'_, DisplacementVertex>> {
         self.displacement_vertices
             .get(n)
             .map(|vert| Handle::new(self, vert))
     }
 
-    pub fn displacement_triangle(&self, n: usize) -> Option<Handle<'_, DisplacementTriangle>> {
-        self.displacement_triangles
-            .get(n)
-            .map(|tri| Handle::new(self, tri))
-    }
-
     /// Get the root node of the bsp
     pub fn root_node(&self) -> Option<Handle<'_, Node>> {
         self.node(0)
@@ -337,6 +332,93 @@ impl Bsp {
     pub fn original_faces(&self) -> impl Iterator<Item = Handle<Face>> {
         self.faces.iter().map(move |face| Handle::new(self, face))
     }
+
+    fn validate(&self) -> BspResult<()> {
+        self.validate_indexes(
+            self.faces
+                .iter()
+                .filter_map(|face| face.displacement_index()),
+            &self.displacements,
+            "face",
+            "displacement",
+        )?;
+        self.validate_indexes(
+            self.displacements
+                .iter()
+                .map(|displacement| displacement.map_face),
+            &self.faces,
+            "displacement",
+            "face",
+        )?;
+        self.validate_indexes(
+            self.faces
+                .iter()
+                .map(|face| face.first_edge + face.num_edges as i32 - 1),
+            &self.surface_edges,
+            "face",
+            "surface_edge",
+        )?;
+        self.validate_indexes(
+            self.surface_edges.iter().map(|edge| edge.edge_index()),
+            &self.edges,
+            "surface_edge",
+            "edge",
+        )?;
+        self.validate_indexes(
+            self.edges
+                .iter()
+                .flat_map(|edge| [edge.start_index, edge.end_index]),
+            &self.vertices,
+            "edge",
+            "vertex",
+        )?;
+        self.validate_indexes(
+            self.displacements
+                .iter()
+                .flat_map(|displacement| &displacement.corner_neighbours)
+                .flat_map(|corner| corner.neighbours()),
+            &self.displacements,
+            "displacement",
+            "displacement",
+        )?;
+        self.validate_indexes(
+            self.displacements
+                .iter()
+                .flat_map(|displacement| &displacement.edge_neighbours)
+                .flat_map(|edge| edge.iter())
+                .map(|sub| sub.neighbour_index),
+            &self.displacements,
+            "displacement",
+            "displacement",
+        )?;
+
+        Ok(())
+    }
+
+    fn validate_indexes<
+        'a,
+        Index: TryInto<usize> + Into<i64> + Copy + Ord + Default,
+        Indexes: Iterator<Item = Index>,
+        T: 'a,
+    >(
+        &'a self,
+        indexes: Indexes,
+        list: &[T],
+        source: &'static str,
+        target: &'static str,
+    ) -> BspResult<()> {
+        let max = indexes.max().unwrap_or_default();
+        max.try_into()
+            .ok()
+            .and_then(|index| list.get(index))
+            .ok_or_else(|| ValidationError::ReferenceOutOfRange {
+                source_: source,
+                target,
+                index: max.into(),
+                size: list.len(),
+            })?;
+        Ok(())
+    }
 }
 
 #[cfg(test)]