Limit the amount of data that can be fetched from untrusted external HTTP resources

9 files changed, 148 insertions, 36 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 5276c5fc..0657b41b 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -5164,10 +5164,12 @@ dependencies = [
  "sync_wrapper 1.0.1",
  "tokio",
  "tokio-rustls 0.26.0",
+ "tokio-util",
  "tower-service",
  "url",
  "wasm-bindgen",
  "wasm-bindgen-futures",
+ "wasm-streams",
  "web-sys",
  "webpki-roots 0.26.6",
  "windows-registry",
diff --git a/crates/common/Cargo.toml b/crates/common/Cargo.toml
index 253424eb..6f30e404 100644
--- a/crates/common/Cargo.toml
+++ b/crates/common/Cargo.toml
@@ -31,7 +31,7 @@ tokio = { version = "1.23", features = ["net", "macros"] }
 tokio-rustls = { version = "0.26", default-features = false, features = ["ring", "tls12"] }
 futures = "0.3"
 rcgen = "0.12"
-reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-webpki-roots", "http2"]}
+reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-webpki-roots", "http2", "stream"]}
 serde = { version = "1.0", features = ["derive"]}
 serde_json = "1.0"
 base64 = "0.22"
diff --git a/crates/common/src/enterprise/mod.rs b/crates/common/src/enterprise/mod.rs
index e4a2230e..69608023 100644
--- a/crates/common/src/enterprise/mod.rs
+++ b/crates/common/src/enterprise/mod.rs
@@ -25,7 +25,7 @@ use store::Store;
 use trc::{AddContext, EventType, MetricType};
 use utils::config::cron::SimpleCron;
 
-use crate::{expr::Expression, manager::webadmin::Resource, Core};
+use crate::{expr::Expression, manager::webadmin::Resource, Core, HttpLimitResponse};
 
 #[derive(Clone)]
 pub struct Enterprise {
@@ -121,6 +121,8 @@ impl Core {
     }
 
     pub async fn logo_resource(&self, domain: &str) -> trc::Result<Option<Resource<Vec<u8>>>> {
+        const MAX_IMAGE_SIZE: usize = 1024 * 1024;
+
         if self.is_enterprise_edition() {
             let domain = psl::domain_str(domain).unwrap_or(domain);
             let logo = { self.security.logos.lock().get(domain).cloned() };
@@ -180,7 +182,7 @@ impl Core {
                         .to_string();
 
                     let contents = response
-                        .bytes()
+                        .bytes_with_limit(MAX_IMAGE_SIZE)
                         .await
                         .map_err(|err| {
                             trc::ResourceEvent::DownloadExternal
@@ -188,7 +190,11 @@ impl Core {
                                 .details("Failed to download logo")
                                 .reason(err)
                         })?
-                        .to_vec();
+                        .ok_or_else(|| {
+                            trc::ResourceEvent::DownloadExternal
+                                .into_err()
+                                .details("Download exceeded maximum size")
+                        })?;
 
                     logo = Resource::new(content_type, contents).into();
                 }
diff --git a/crates/common/src/lib.rs b/crates/common/src/lib.rs
index d602a1ca..e8dbf60e 100644
--- a/crates/common/src/lib.rs
+++ b/crates/common/src/lib.rs
@@ -30,6 +30,7 @@ use directory::{
     Principal, QueryBy, Type,
 };
 use expr::if_block::IfBlock;
+use futures::StreamExt;
 use listener::{
     blocked::{AllowedIps, BlockedIps},
     tls::TlsManager,
@@ -38,6 +39,7 @@ use mail_send::Credentials;
 
 use manager::webadmin::Resource;
 use parking_lot::Mutex;
+use reqwest::Response;
 use sieve::Sieve;
 use store::{
     write::{QueueClass, ValueClass},
@@ -415,3 +417,34 @@ impl Clone for Security {
         }
     }
 }
+
+pub trait HttpLimitResponse: Sync + Send {
+    fn bytes_with_limit(
+        self,
+        limit: usize,
+    ) -> impl std::future::Future<Output = reqwest::Result<Option<Vec<u8>>>> + Send;
+}
+
+impl HttpLimitResponse for Response {
+    async fn bytes_with_limit(self, limit: usize) -> reqwest::Result<Option<Vec<u8>>> {
+        if self
+            .content_length()
+            .map_or(false, |len| len as usize > limit)
+        {
+            return Ok(None);
+        }
+
+        let mut bytes = Vec::with_capacity(std::cmp::min(limit, 1024));
+        let mut stream = self.bytes_stream();
+
+        while let Some(chunk) = stream.next().await {
+            let chunk = chunk?;
+            if bytes.len() + chunk.len() > limit {
+                return Ok(None);
+            }
+            bytes.extend_from_slice(&chunk);
+        }
+
+        Ok(Some(bytes))
+    }
+}
diff --git a/crates/common/src/scripts/plugins/lookup.rs b/crates/common/src/scripts/plugins/lookup.rs
index 404961ed..f19721c5 100644
--- a/crates/common/src/scripts/plugins/lookup.rs
+++ b/crates/common/src/scripts/plugins/lookup.rs
@@ -14,7 +14,9 @@ use mail_auth::flate2;
 use sieve::{runtime::Variable, FunctionMap};
 use store::{Deserialize, Value};
 
-use crate::{config::scripts::RemoteList, scripts::into_sieve_value, USER_AGENT};
+use crate::{
+    config::scripts::RemoteList, scripts::into_sieve_value, HttpLimitResponse, USER_AGENT,
+};
 
 use super::PluginContext;
 
@@ -144,6 +146,8 @@ pub async fn exec_remote(ctx: PluginContext<'_>) -> trc::Result<Variable> {
     }
 }
 
+const MAX_RESOURCE_SIZE: usize = 10 * 1024 * 1024;
+
 async fn exec_remote_(ctx: &PluginContext<'_>) -> trc::Result<Variable> {
     let resource = ctx.arguments[0].to_string();
     let item = ctx.arguments[1].to_string();
@@ -228,13 +232,22 @@ async fn exec_remote_(ctx: &PluginContext<'_>) -> trc::Result<Variable> {
         })?;
 
     if response.status().is_success() {
-        let bytes = response.bytes().await.map_err(|err| {
-            trc::SieveEvent::RuntimeError
-                .into_err()
-                .reason(err)
-                .ctx(trc::Key::Url, resource.to_string())
-                .details("Failed to fetch resource")
-        })?;
+        let bytes = response
+            .bytes_with_limit(MAX_RESOURCE_SIZE)
+            .await
+            .map_err(|err| {
+                trc::SieveEvent::RuntimeError
+                    .into_err()
+                    .reason(err)
+                    .ctx(trc::Key::Url, resource.to_string())
+                    .details("Failed to fetch resource")
+            })?
+            .ok_or_else(|| {
+                trc::SieveEvent::RuntimeError
+                    .into_err()
+                    .ctx(trc::Key::Url, resource.to_string())
+                    .details("Resource is too large")
+            })?;
 
         let reader: Box<dyn std::io::Read> = if resource.ends_with(".gz") {
             Box::new(flate2::read::GzDecoder::new(&bytes[..]))
diff --git a/crates/directory/src/core/principal.rs b/crates/directory/src/core/principal.rs
index 5d134e73..f6dc00f6 100644
--- a/crates/directory/src/core/principal.rs
+++ b/crates/directory/src/core/principal.rs
@@ -608,6 +608,8 @@ impl serde::Serialize for Principal {
     }
 }
 
+const MAX_STRING_LEN: usize = 512;
+
 impl<'de> serde::Deserialize<'de> for PrincipalValue {
     fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
     where
@@ -647,14 +649,22 @@ impl<'de> serde::Deserialize<'de> for PrincipalValue {
             where
                 E: de::Error,
             {
-                Ok(PrincipalValue::String(value))
+                if value.len() <= MAX_STRING_LEN {
+                    Ok(PrincipalValue::String(value))
+                } else {
+                    Err(serde::de::Error::custom("string too long"))
+                }
             }
 
-            fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+            fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
             where
                 E: de::Error,
             {
-                Ok(PrincipalValue::String(v.to_string()))
+                if value.len() <= MAX_STRING_LEN {
+                    Ok(PrincipalValue::String(value.to_string()))
+                } else {
+                    Err(serde::de::Error::custom("string too long"))
+                }
             }
 
             fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
@@ -666,7 +676,13 @@ impl<'de> serde::Deserialize<'de> for PrincipalValue {
 
                 while let Some(value) = seq.next_element::<StringOrU64>()? {
                     match value {
-                        StringOrU64::String(s) => vec_string.push(s),
+                        StringOrU64::String(s) => {
+                            if s.len() <= MAX_STRING_LEN {
+                                vec_string.push(s);
+                            } else {
+                                return Err(serde::de::Error::custom("string too long"));
+                            }
+                        }
                         StringOrU64::U64(u) => vec_u64.push(u),
                     }
                 }
@@ -720,12 +736,24 @@ impl<'de> serde::Deserialize<'de> for Principal {
                         })?;
 
                     let value = match key {
-                        PrincipalField::Name => PrincipalValue::String(map.next_value()?),
+                        PrincipalField::Name => {
+                            PrincipalValue::String(map.next_value::<String>().and_then(|v| {
+                                if v.len() <= MAX_STRING_LEN {
+                                    Ok(v)
+                                } else {
+                                    Err(serde::de::Error::custom("string too long"))
+                                }
+                            })?)
+                        }
                         PrincipalField::Description
                         | PrincipalField::Tenant
                         | PrincipalField::Picture => {
                             if let Some(v) = map.next_value::<Option<String>>()? {
-                                PrincipalValue::String(v)
+                                if v.len() <= MAX_STRING_LEN {
+                                    PrincipalValue::String(v)
+                                } else {
+                                    return Err(serde::de::Error::custom("string too long"));
+                                }
                             } else {
                                 continue;
                             }
@@ -798,7 +826,22 @@ impl<'de> serde::Deserialize<'de> for StringOrU64 {
             where
                 E: de::Error,
             {
-                Ok(StringOrU64::String(value.to_string()))
+                if value.len() <= MAX_STRING_LEN {
+                    Ok(StringOrU64::String(value.to_string()))
+                } else {
+                    Err(serde::de::Error::custom("string too long"))
+                }
+            }
+
+            fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                if v.len() <= MAX_STRING_LEN {
+                    Ok(StringOrU64::String(v))
+                } else {
+                    Err(serde::de::Error::custom("string too long"))
+                }
             }
 
             fn visit_u64<E>(self, value: u64) -> Result<Self::Value, E>
@@ -837,7 +880,22 @@ impl<'de> serde::Deserialize<'de> for StringOrMany {
             where
                 E: de::Error,
             {
-                Ok(StringOrMany::One(value.to_string()))
+                if value.len() <= MAX_STRING_LEN {
+                    Ok(StringOrMany::One(value.to_string()))
+                } else {
+                    Err(serde::de::Error::custom("string too long"))
+                }
+            }
+
+            fn visit_string<E>(self, v: String) -> Result<Self::Value, E>
+            where
+                E: de::Error,
+            {
+                if v.len() <= MAX_STRING_LEN {
+                    Ok(StringOrMany::One(v))
+                } else {
+                    Err(serde::de::Error::custom("string too long"))
+                }
             }
 
             fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
diff --git a/crates/directory/src/lib.rs b/crates/directory/src/lib.rs
index 9d767e69..fc5474c0 100644
--- a/crates/directory/src/lib.rs
+++ b/crates/directory/src/lib.rs
@@ -59,6 +59,8 @@ pub enum Type {
 )]
 #[serde(rename_all = "kebab-case")]
 pub enum Permission {
+    // WARNING: add new ids at the end (TODO: use static ids)
+
     // Admin
     Impersonate,
     UnlimitedRequests,
@@ -238,6 +240,7 @@ pub enum Permission {
     SieveRenameScript,
     SieveCheckScript,
     SieveHaveSpace,
+    // WARNING: add new ids at the end (TODO: use static ids)
 }
 
 pub type Permissions = Bitset<
diff --git a/crates/smtp/src/inbound/hooks/client.rs b/crates/smtp/src/inbound/hooks/client.rs
index 0aa3f2c7..e915bd81 100644
--- a/crates/smtp/src/inbound/hooks/client.rs
+++ b/crates/smtp/src/inbound/hooks/client.rs
@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-SEL
  */
 
-use common::config::smtp::session::MTAHook;
+use common::{config::smtp::session::MTAHook, HttpLimitResponse};
 
 use super::{Request, Response};
 
@@ -28,22 +28,12 @@ pub(super) async fn send_mta_hook_request(
         .map_err(|err| format!("Hook request failed: {err}"))?;
 
     if response.status().is_success() {
-        if response
-            .content_length()
-            .map_or(false, |len| len as usize > mta_hook.max_response_size)
-        {
-            return Err(format!(
-                "Hook response too large ({} bytes)",
-                response.content_length().unwrap()
-            ));
-        }
-
-        // TODO: Stream response body to limit response size
         serde_json::from_slice(
             response
-                .bytes()
+                .bytes_with_limit(mta_hook.max_response_size)
                 .await
                 .map_err(|err| format!("Failed to parse Hook response: {}", err))?
+                .ok_or_else(|| "Hook response too large".to_string())?
                 .as_ref(),
         )
         .map_err(|err| format!("Failed to parse Hook response: {}", err))
diff --git a/crates/smtp/src/outbound/mta_sts/lookup.rs b/crates/smtp/src/outbound/mta_sts/lookup.rs
index 58d4b93d..38fe9164 100644
--- a/crates/smtp/src/outbound/mta_sts/lookup.rs
+++ b/crates/smtp/src/outbound/mta_sts/lookup.rs
@@ -20,6 +20,12 @@ use crate::core::SMTP;
 
 use super::{parse::ParsePolicy, Error};
 
+#[cfg(not(feature = "test_mode"))]
+use common::HttpLimitResponse;
+
+#[cfg(not(feature = "test_mode"))]
+const MAX_POLICY_SIZE: usize = 1024 * 1024;
+
 #[allow(unused_variables)]
 impl SMTP {
     pub async fn lookup_mta_sts_policy<'x>(
@@ -61,11 +67,12 @@ impl SMTP {
             .timeout(timeout)
             .redirect(reqwest::redirect::Policy::none())
             .build()?
-            .get(&format!("https://mta-sts.{domain}/.well-known/mta-sts.txt"))
+            .get(format!("https://mta-sts.{domain}/.well-known/mta-sts.txt"))
             .send()
             .await?
-            .bytes()
-            .await?;
+            .bytes_with_limit(MAX_POLICY_SIZE)
+            .await?
+            .ok_or_else(|| Error::InvalidPolicy("Policy too large".to_string()))?;
         #[cfg(feature = "test_mode")]
         let bytes = STS_TEST_POLICY.lock().clone();