From c523ffe8f6bc12f69863aa5d688ffc302472edfa Mon Sep 17 00:00:00 2001
From: Nikolaus Waxweiler <nikolaus.waxweiler@daltonmaag.com>
Date: Tue, 11 Jan 2022 12:20:46 +0000
Subject: [PATCH] Protect Image.file_name

It must ensure some invariants.
---
 src/glyph/mod.rs       | 30 ++++++++++++++++++++++++++++--
 src/glyph/parse.rs     |  5 ++++-
 src/glyph/serialize.rs |  2 +-
 3 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/glyph/mod.rs b/src/glyph/mod.rs
index 22e3e7e1..5d4ddbe6 100644
--- a/src/glyph/mod.rs
+++ b/src/glyph/mod.rs
@@ -14,7 +14,7 @@ use crate::error::ConvertContourError;
 #[cfg(feature = "druid")]
 use druid::{Data, Lens};
 
-use crate::error::{ErrorKind, GlifLoadError, GlifWriteError};
+use crate::error::{ErrorKind, GlifLoadError, GlifWriteError, StoreError};
 use crate::name::Name;
 use crate::names::NameList;
 use crate::shared_types::PUBLIC_OBJECT_LIBS_KEY;
@@ -735,13 +735,39 @@ impl std::default::Default for AffineTransform {
 #[derive(Debug, Clone, PartialEq)]
 pub struct Image {
     /// The name of the image file. Must be a base file name, no subdirectories involved.
-    pub file_name: PathBuf,
+    file_name: PathBuf,
     /// Optional image color.
     pub color: Option<Color>,
     /// Affine transformation.
     pub transform: AffineTransform,
 }
 
+impl Image {
+    /// Create a new image.
+    pub fn new(
+        file_name: PathBuf,
+        color: Option<Color>,
+        transform: AffineTransform,
+    ) -> Result<Self, StoreError> {
+        // Note: Mostly mirrors [`Self::validate_entry`].
+        if file_name.as_os_str().is_empty() {
+            return Err(StoreError::EmptyPath);
+        }
+        if file_name.is_absolute() {
+            return Err(StoreError::PathIsAbsolute);
+        }
+        if file_name.parent().map_or(false, |p| !p.as_os_str().is_empty()) {
+            return Err(StoreError::Subdir);
+        }
+        Ok(Self { file_name, color, transform })
+    }
+
+    /// Returns the file name of the image.
+    pub fn file_name(&self) -> &Path {
+        self.file_name.as_path()
+    }
+}
+
 #[cfg(feature = "kurbo")]
 impl From<AffineTransform> for kurbo::Affine {
     fn from(src: AffineTransform) -> kurbo::Affine {
diff --git a/src/glyph/parse.rs b/src/glyph/parse.rs
index dd979729..fae03d37 100644
--- a/src/glyph/parse.rs
+++ b/src/glyph/parse.rs
@@ -552,7 +552,10 @@ impl<'names> GlifParser<'names> {
 
         match filename {
             Some(file_name) => {
-                self.glyph.image = Some(Image { file_name, color, transform });
+                self.glyph.image = Some(
+                    Image::new(file_name, color, transform)
+                        .map_err(|_| GlifLoadError::Parse(ErrorKind::BadImage))?,
+                );
                 Ok(())
             }
             None => Err(ErrorKind::BadImage.into()),
diff --git a/src/glyph/serialize.rs b/src/glyph/serialize.rs
index 86a5f1ad..a5bf57e1 100644
--- a/src/glyph/serialize.rs
+++ b/src/glyph/serialize.rs
@@ -347,7 +347,7 @@ impl Color {
 impl Image {
     fn to_event(&self) -> Event {
         let mut start = BytesStart::borrowed_name(b"image");
-        start.push_attribute(("fileName", self.file_name.to_str().unwrap_or("missing path")));
+        start.push_attribute(("fileName", self.file_name.to_str().expect("missing path")));
 
         write_transform_attributes(&mut start, &self.transform);