Merge pull request #535 from storytold/fix/sec-fontconfig-include-dirs

font-catalog: list each fontconfig include folder once, within a budget

crates/font-catalog/src/fontconfig.rs

@@ -1,7 +1,7 @@
 //! The font folders named by the fontconfig configuration, read with a small tag scanner (no
 //! fontconfig library, no XML crate): only `<dir>`, `<remap-dir>` and `<include>` matter here.
-//! The work is bounded: include depth, number and size of the files read, and every file is read
-//! at most once.
+//! The work is bounded: include depth, number and size of the files read, the folder entries
+//! listed for includes, and every file is read and every folder listed at most once.
 
 use std::collections::HashSet;
 use std::path::{Path, PathBuf};
@@ -11,6 +11,8 @@ use crate::FontEnv;
 const MAX_INCLUDE_DEPTH: u32 = 8;
 const MAX_FILES: usize = 512;
 const MAX_FILE_BYTES: u64 = 1 << 20;
+/// Folder entries listed for `<include>` folders, all folders together.
+const MAX_DIR_ENTRIES: usize = 16_384;
 
 /// The folders the fontconfig configuration for `env` names, in the order it names them.
 ///
@@ -21,7 +23,7 @@ const MAX_FILE_BYTES: u64 = 1 << 20;
 /// those `/run/host/font-dirs.xml` maps, and those of the host's own configuration when the
 /// sandbox can see it (`/run/host/etc/fonts/fonts.conf`), under `/run/host`.
 pub fn fontconfig_dirs(env: &FontEnv) -> Vec<PathBuf> {
-    let mut reader = Reader { env, out: Vec::new(), seen: HashSet::new(), host: None };
+    let mut reader = Reader { env, out: Vec::new(), seen: HashSet::new(), dirs: HashSet::new(), entries_left: MAX_DIR_ENTRIES, host: None };
     let root = match &env.fontconfig_file {
         Some(f) => Some(f.clone()),
         None => ["/etc/fonts/fonts.conf", "/usr/local/etc/fonts/fonts.conf"].map(|p| env.rooted(p)).into_iter().find(|p| p.is_file()),
@@ -45,6 +47,10 @@ struct Reader<'a> {
     env: &'a FontEnv,
     out: Vec<PathBuf>,
     seen: HashSet<PathBuf>,
+    /// The `<include>` folders already listed (canonical paths).
+    dirs: HashSet<PathBuf>,
+    /// Folder entries that may still be listed ([`MAX_DIR_ENTRIES`]).
+    entries_left: usize,
     /// While reading the host's configuration from a sandbox: where the host's root is.
     host: Option<PathBuf>,
 }
@@ -53,14 +59,27 @@ impl Reader<'_> {
     /// Read a configuration file or, for a folder, its `NN-name.conf` files in name order.
     fn include(&mut self, path: &Path, depth: u32) {
         if path.is_dir() {
+            // Check the budgets before listing: a configuration that names the same folder many
+            // times, or many large folders, must not list them over and over.
+            if depth > MAX_INCLUDE_DEPTH || self.seen.len() >= MAX_FILES || self.entries_left == 0 {
+                return;
+            }
+            let key = std::fs::canonicalize(path).unwrap_or_else(|_| path.to_path_buf());
+            if !self.dirs.insert(key) {
+                return;
+            }
             let Ok(rd) = std::fs::read_dir(path) else { return };
+            let mut listed = 0;
             let mut files: Vec<PathBuf> = rd
+                .take(self.entries_left)
+                .inspect(|_| listed += 1)
                 .flatten()
                 .map(|e| e.path())
                 .filter(|p| {
                     p.file_name().and_then(|n| n.to_str()).is_some_and(|n| n.ends_with(".conf") && n.starts_with(|c: char| c.is_ascii_digit()))
                 })
                 .collect();
+            self.entries_left -= listed;
             files.sort();
             for f in files {
                 self.file(&f, depth);
@@ -191,3 +210,28 @@ fn elements(src: &str) -> Vec<Element<'_>> {
 fn decode_entities(s: &str) -> String {
     s.replace("&lt;", "<").replace("&gt;", ">").replace("&quot;", "\"").replace("&apos;", "'").replace("&amp;", "&")
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn a_folder_included_many_times_is_listed_once() {
+        let root = std::env::temp_dir().join(format!("craft-font-catalog-fc-dirs-{}", std::process::id()));
+        let _ = std::fs::remove_dir_all(&root);
+        let conf_d = root.join("conf.d");
+        std::fs::create_dir_all(&conf_d).unwrap();
+        for (i, dir) in ["/a", "/b", "/c"].iter().enumerate() {
+            std::fs::write(conf_d.join(format!("{i}0-x.conf")), format!("<fontconfig><dir>{dir}</dir></fontconfig>")).unwrap();
+        }
+        let main = root.join("fonts.conf");
+        std::fs::write(&main, format!("<fontconfig>{}</fontconfig>", "<include>conf.d</include>".repeat(5000))).unwrap();
+        let env = FontEnv { root: root.clone(), ..Default::default() };
+        let mut reader = Reader { env: &env, out: Vec::new(), seen: HashSet::new(), dirs: HashSet::new(), entries_left: MAX_DIR_ENTRIES, host: None };
+        reader.file(&main, 0);
+        assert_eq!(reader.out, vec![root.join("a"), root.join("b"), root.join("c")]);
+        assert_eq!(reader.dirs.len(), 1);
+        assert_eq!(reader.entries_left, MAX_DIR_ENTRIES - 3);
+        let _ = std::fs::remove_dir_all(&root);
+    }
+}