Fix(api): Fixed several vulnerabilities in Thorium

None of these issues allow for RCE or privilege escalation.

Result File Path Normalization

The API was not validating that uploaded result file paths are not
absolute paths and do not contain any '..' components. This was not
exploitable due to the fact that:
- Some s3 servers (Minio and CEPH were tested) do not allow .. in paths
- The agent panics when downloading files with an absolute path
- Thorctl nests the absolute path in its relative path and returns an
error Regardless this has been resolved, and Thorium will now validate
and reject any absolute paths or paths where any component contains only
'.'s.

LDAP Injection

Thorium was not escaping user controlled strings that it sent to LDAP.
This would allow attackers to perform LDAP injection if they can add
metagroups to groups. In order to perform this attack, an attacker
would already have the permissions to modify group permissions at will.
Thorium now properly escapes user controlled strings in ldap.

Spam Verification Emails For Unverified Users

Thorium was not limiting how often verification emails could be resent
to unverified users in systems that have email verification configured.
This means that if an attacker knew a user's username and that user had
not yet verified their email, they could spam them with emails. Only the
verification email would sent this does not allow an attacker to send
arbitrary emails. Thorium now allows admins to set a rate limit value
that currently defaults to only allowing an email to be resent every 10
minutes.

Token Not Rotating When Resetting Passwords

Thorium was generating a new token but not saving it when updating a
users password. This meant that if a user was updating their password
due to a password or token being leaked, Thorium did not properly remove
all prior access. This is only relevant to LDAP enabled Thorium clusters.
Thorium now saves the new token on password updates.

Disabled TLS Verification To Elasticsearch

Thorium was not allowing users to configure how they want to validate
the certificate used by elastic search and was defaulting to not
verifying it. This option is now configurable.

Divide By Zero When Getting Streams

If a user set a split of 0 when getting streams, that request would panic
due to a divide by zero error. This has been resolved by requiring a
NonZeroU64 instead of a u64.

Thanks to OpenAI Security Research for bringing these issues to our attention.

Author: mjcarson

Cargo.lock

@@ -16,7 +16,7 @@ members = [
 ]
 
 [workspace.package]
-version = "1.1.0"
+version = "1.1.1"
 authors = ["mcarson <mcarson@sandia.gov>", "gmbaker <gmbaker@sandia.gov>", "jehamza <jehamza@sandia.gov>"]
 edition = "2024"
 

Cargo.toml

@@ -36,14 +36,14 @@ infer = { version = "0.19.0", default-features = false, features = ["std"] }
 
 # enable cgroups support for linux
 [target.'cfg(target_os = "linux")'.dependencies]
-thorium = { version = "1.1.0", path="../api", default-features = false, features = ["client", "cgroups", "crossbeam-err", "trace"]}
+thorium = { version = "1.1.1", path="../api", default-features = false, features = ["client", "cgroups", "crossbeam-err", "trace"]}
 cgroups-rs = "0.3"
 controlgroup = "0.3"
 
 
 # disable cgroups support when on windows
 [target.'cfg(any(target_os = "windows", target_os = "macos"))'.dependencies]
-thorium = { version = "1.1.0", path="../api", default-features = false, features = ["client", "crossbeam-err", "trace"]}
+thorium = { version = "1.1.1", path="../api", default-features = false, features = ["client", "crossbeam-err", "trace"]}
 
 
 [features]

agent/Cargo.toml

@@ -2,7 +2,7 @@ use clap::Parser;
 use serde_derive::Deserialize;
 use thorium::models::ImageScaler;
 use thorium::{Error, Thorium};
-use tracing::{event, instrument, Level};
+use tracing::{Level, event, instrument};
 
 use crate::libs::Target;
 
@@ -37,6 +37,9 @@ pub struct Args {
     /// The keys to use when authenticating to Thorium
     #[clap(short, long, default_value = "keys.yml")]
     pub keys: String,
+    /// How long should this agent sit limbo before exiting without a job to work on
+    #[clap(short, long, default_value = "30")]
+    pub limbo: usize,
 }
 
 impl Args {
@@ -103,7 +106,7 @@ impl Args {
                         return Err(Error::new(format!(
                             "Failed to get hostname with {:#?}",
                             err
-                        )))
+                        )));
                     }
                 }
             }

agent/src/args.rs

@@ -1,9 +1,9 @@
 use futures::{poll, task::Poll};
 use std::time::Duration;
-use thorium::models::{StageLogsAdd, WorkerStatus};
 use thorium::Error;
 use thorium::Thorium;
-use tracing::{event, instrument, span, Level};
+use thorium::models::{StageLogsAdd, WorkerStatus};
+use tracing::{Level, event, instrument, span};
 
 use super::agents::{self, Agent};
 use super::{CurrentTarget, Lifetime, Target};

agent/src/libs/worker.rs

@@ -134,7 +134,7 @@ once_cell = { version = "1.21.3", optional = true }
 utoipa = { version = "5", features = ["axum_extras", "chrono", "uuid", "time", "url"], optional = true }
 utoipa-swagger-ui = { version = "9", features = ["axum"], optional = true }
 lettre = { version = "0.11", features = ["tokio1", "tokio1-rustls-tls", "builder", "smtp-transport"], default-features = false, optional = true }
-thorium-derive = { path = "../thorium-derive", version = "1.1.0", optional = true}
+thorium-derive = { path = "../thorium-derive", version = "1.1.1", optional = true}
 
 # rkyv dependencies
 rkyv = { version = "=0.7.43", features = ["arbitrary_enum_discriminant", "uuid", "validation"], optional = true }

api/Cargo.toml

@@ -278,17 +278,9 @@ pub struct Ldap {
     pub credentials: Option<LdapCreds>,
 }
 
-/// Helps serde default the default user token expiration to 90
-fn default_token_expire() -> u32 {
-    90
-}
-
-/// Helps serde default the local user/group ids to a sane default
-fn default_local_user_ids() -> UnixInfo {
-    UnixInfo {
-        user: 1_879_048_192,
-        group: 1_879_048_192,
-    }
+/// Help serde default the email rate limit to 10 minutes
+fn default_email_rate_limit() -> u64 {
+    600
 }
 
 /// The email settings to use for verification emails
@@ -304,6 +296,54 @@ pub struct EmailVerification {
     pub password: String,
     /// The email regexes to restrict users too
     pub approved_emails: Vec<String>,
+    /// The time to require between verification emails in seconds
+    #[serde(default = "default_email_rate_limit")]
+    pub rate_limit: u64,
+}
+
+impl EmailVerification {
+    /// Check if a user has recently sent a verification email
+    ///
+    /// This will return an error if we can't send a verification email yet.
+    ///
+    /// # Arguments
+    ///
+    /// * `user` - The user that we might send a verification email too
+    #[cfg(feature = "api")]
+    pub fn can_send_verification(
+        &self,
+        user: &crate::models::User,
+    ) -> Result<(), crate::utils::ApiError> {
+        // get the last time an email was sent if it was
+        if let Some(sent) = user.verification_sent {
+            // calculate the time at which our rate limit no longer applies
+            let limit_ts = chrono::Utc::now() - chrono::Duration::seconds(self.rate_limit as i64);
+            // a previous verification email was sent so check our rate limit
+            if sent > limit_ts {
+                // calculate when we can next send a verification email
+                let wait_time = sent - limit_ts;
+                // return a 429
+                return Err(crate::too_many_requests!(format!(
+                    "Cannot send verificaiton for another {} seconds",
+                    wait_time.num_seconds()
+                )));
+            }
+        }
+        Ok(())
+    }
+}
+
+/// Helps serde default the default user token expiration to 90
+fn default_token_expire() -> u32 {
+    90
+}
+
+/// Helps serde default the local user/group ids to a sane default
+fn default_local_user_ids() -> UnixInfo {
+    UnixInfo {
+        user: 1_879_048_192,
+        group: 1_879_048_192,
+    }
 }
 
 /// Authentication settings

api/src/conf.rs

@@ -105,6 +105,7 @@ async fn initial_settings_consistency_scan(
         settings: crate::models::UserSettings::default(),
         verified: bool::default(),
         verification_token: None,
+        verification_sent: None,
     };
     // do a scan for consistency according to current settings
     settings.consistency_scan(&fake_admin, &shared).await?;
@@ -130,7 +131,7 @@ fn build_app(
     use std::time::Duration;
     use tower_http::set_header::SetResponseHeaderLayer;
     use tower_http::trace::{DefaultMakeSpan, TraceLayer};
-    use tracing::{event, Level, Span};
+    use tracing::{Level, Span, event};
 
     use crate::utils::trace;
 

api/src/lib.rs

@@ -1,4 +1,5 @@
 use bb8_redis::redis::cmd;
+use chrono::prelude::*;
 use std::collections::{HashMap, HashSet};
 use tracing::instrument;
 
@@ -116,6 +117,7 @@ pub(super) fn cast(
         settings: deserialize_ext!(raw, "settings", UserSettings::default()),
         verified: helpers::extract_bool_default(&mut raw, "verified", true)?,
         verification_token: helpers::extract_opt(&mut raw, "verification_token"),
+        verification_sent: deserialize_opt!(&mut raw, "verification_sent"),
     };
     Ok(user)
 }
@@ -426,7 +428,8 @@ pub async fn set_verification_token(username: &str, verification_token: &str, sh
     // build a redis pipeline
     let mut pipe = redis::pipe();
     // set our updated verification token
-    pipe.cmd("hset").arg(&data_key).arg("verification_token").arg(verification_token);
+    pipe.cmd("hset").arg(&data_key).arg("verification_token").arg(verification_token)
+        .cmd("hset").arg(&data_key).arg("verification_sent").arg(serialize!(&Utc::now()));
     // save user into redis
     let _: () = pipe.atomic()
         .query_async(conn!(shared))

api/src/models/backends/db/users.rs

@@ -1004,7 +1004,7 @@ impl
 macro_rules! build_filter {
     ($arr:expr, $key:expr, $target:expr) => {
         for item in $arr.iter() {
-            $target.insert(format!("({}={})", $key, item));
+            $target.insert(format!("({}={})", $key, ldap3::ldap_escape(item)));
         }
     };
 }