diff --git a/Cargo.lock b/Cargo.lock index 3ae0af05..72e96679 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,7 +60,7 @@ dependencies = [ [[package]] name = "age" -version = "0.11.0" +version = "0.11.1" dependencies = [ "aes", "aes-gcm", @@ -2042,7 +2042,7 @@ dependencies = [ [[package]] name = "rage" -version = "0.11.0" +version = "0.11.1" dependencies = [ "age", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 40b27a58..8468db45 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ repository = "https://github.com/str4d/rage" license = "MIT OR Apache-2.0" [workspace.dependencies] -age = { version = "0.11.0", path = "age" } +age = { version = "0.11.1", path = "age" } age-core = { version = "0.11.0", path = "age-core" } # Dependencies required by the age specification: diff --git a/age/CHANGELOG.md b/age/CHANGELOG.md index 9cc91785..eb79b409 100644 --- a/age/CHANGELOG.md +++ b/age/CHANGELOG.md @@ -10,6 +10,13 @@ to 1.0.0 are beta releases. ## [Unreleased] +## [0.6.1, 0.7.2, 0.8.2, 0.9.3, 0.10.1, 0.11.1] - 2024-11-18 +### Security +- Fixed a security vulnerability that could allow an attacker to execute an + arbitrary binary under certain conditions. See GHSA-4fg7-vxc8-qx5w. Plugin + names are now required to only contain alphanumeric characters or the four + special characters `+-._`. Thanks to ⬡-49016 for reporting this issue. + ## [0.11.0] - 2024-11-03 ### Added - New streamlined APIs for use with a single recipient or identity and a small diff --git a/age/Cargo.toml b/age/Cargo.toml index 720270c1..11510461 100644 --- a/age/Cargo.toml +++ b/age/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "age" description = "[BETA] A simple, secure, and modern encryption library." -version = "0.11.0" +version = "0.11.1" authors.workspace = true repository.workspace = true readme = "README.md" diff --git a/age/src/plugin.rs b/age/src/plugin.rs index fc1af5d9..f38ec0cf 100644 --- a/age/src/plugin.rs +++ b/age/src/plugin.rs @@ -43,6 +43,13 @@ const CMD_FILE_KEY: &str = "file-key"; const ONE_HUNDRED_MS: Duration = Duration::from_millis(100); const TEN_SECONDS: Duration = Duration::from_secs(10); +#[inline] +fn valid_plugin_name(plugin_name: &str) -> bool { + plugin_name + .bytes() + .all(|b| b.is_ascii_alphanumeric() | matches!(b, b'+' | b'-' | b'.' | b'_')) +} + fn binary_name(plugin_name: &str) -> String { format!("age-plugin-{}", plugin_name) } @@ -104,10 +111,15 @@ impl std::str::FromStr for Recipient { if hrp.len() > PLUGIN_RECIPIENT_PREFIX.len() && hrp.starts_with(PLUGIN_RECIPIENT_PREFIX) { - Ok(Recipient { - name: hrp.split_at(PLUGIN_RECIPIENT_PREFIX.len()).1.to_owned(), - recipient: s.to_owned(), - }) + let name = hrp.split_at(PLUGIN_RECIPIENT_PREFIX.len()).1.to_owned(); + if valid_plugin_name(&name) { + Ok(Recipient { + name, + recipient: s.to_owned(), + }) + } else { + Err("invalid plugin name") + } } else { Err("invalid HRP") } @@ -148,14 +160,20 @@ impl std::str::FromStr for Identity { if hrp.len() > PLUGIN_IDENTITY_PREFIX.len() && hrp.starts_with(PLUGIN_IDENTITY_PREFIX) { - Ok(Identity { - name: hrp - .split_at(PLUGIN_IDENTITY_PREFIX.len()) - .1 - .trim_end_matches('-') - .to_owned(), - identity: s.to_owned(), - }) + // TODO: Decide whether to allow plugin names to end in - + let name = hrp + .split_at(PLUGIN_IDENTITY_PREFIX.len()) + .1 + .trim_end_matches('-') + .to_owned(); + if valid_plugin_name(&name) { + Ok(Identity { + name, + identity: s.to_owned(), + }) + } else { + Err("invalid plugin name") + } } else { Err("invalid HRP") } @@ -171,16 +189,25 @@ impl fmt::Display for Identity { impl Identity { /// Returns the identity corresponding to the given plugin name in its default mode. + /// + /// # Panics + /// + /// Panics if `plugin_name` contains invalid characters. pub fn default_for_plugin(plugin_name: &str) -> Self { - bech32::encode( - &format!("{}{}-", PLUGIN_IDENTITY_PREFIX, plugin_name), - [], - Variant::Bech32, - ) - .expect("HRP is valid") - .to_uppercase() - .parse() - .unwrap() + if valid_plugin_name(plugin_name) { + bech32::encode( + &format!("{}{}-", PLUGIN_IDENTITY_PREFIX, plugin_name), + [], + Variant::Bech32, + ) + .expect("HRP is valid") + .to_uppercase() + .parse() + .unwrap() + } else { + // TODO: Change the API to be fallible. + panic!("invalid plugin name") + } } /// Returns the plugin name for this identity. @@ -359,22 +386,28 @@ impl RecipientPluginV1 { identities: &[Identity], callbacks: C, ) -> Result { - Plugin::new(plugin_name) - .map_err(|binary_name| EncryptError::MissingPlugin { binary_name }) - .map(|plugin| RecipientPluginV1 { - plugin, - recipients: recipients - .iter() - .filter(|r| r.name == plugin_name) - .cloned() - .collect(), - identities: identities - .iter() - .filter(|r| r.name == plugin_name) - .cloned() - .collect(), - callbacks, + if valid_plugin_name(plugin_name) { + Plugin::new(plugin_name) + .map_err(|binary_name| EncryptError::MissingPlugin { binary_name }) + .map(|plugin| RecipientPluginV1 { + plugin, + recipients: recipients + .iter() + .filter(|r| r.name == plugin_name) + .cloned() + .collect(), + identities: identities + .iter() + .filter(|r| r.name == plugin_name) + .cloned() + .collect(), + callbacks, + }) + } else { + Err(EncryptError::MissingPlugin { + binary_name: plugin_name.to_string(), }) + } } } @@ -563,16 +596,22 @@ impl IdentityPluginV1 { identities: &[Identity], callbacks: C, ) -> Result { - Plugin::new(plugin_name) - .map_err(|binary_name| DecryptError::MissingPlugin { binary_name }) - .map(|plugin| { - let identities = identities - .iter() - .filter(|r| r.name == plugin_name) - .cloned() - .collect(); - Self::from_parts(plugin, identities, callbacks) + if valid_plugin_name(plugin_name) { + Plugin::new(plugin_name) + .map_err(|binary_name| DecryptError::MissingPlugin { binary_name }) + .map(|plugin| { + let identities = identities + .iter() + .filter(|r| r.name == plugin_name) + .cloned() + .collect(); + Self::from_parts(plugin, identities, callbacks) + }) + } else { + Err(DecryptError::MissingPlugin { + binary_name: plugin_name.to_string(), }) + } } pub(crate) fn from_parts(plugin: Plugin, identities: Vec, callbacks: C) -> Self { @@ -697,7 +736,14 @@ impl crate::Identity for IdentityPluginV1 { #[cfg(test)] mod tests { - use super::Identity; + use crate::{DecryptError, EncryptError, NoCallbacks}; + + use super::{ + Identity, IdentityPluginV1, Recipient, RecipientPluginV1, PLUGIN_IDENTITY_PREFIX, + PLUGIN_RECIPIENT_PREFIX, + }; + + const INVALID_PLUGIN_NAME: &str = "foobar/../../../../../../../usr/bin/echo"; #[test] fn default_for_plugin() { @@ -706,4 +752,49 @@ mod tests { "AGE-PLUGIN-FOOBAR-1QVHULF", ); } + + #[test] + fn recipient_rejects_invalid_chars() { + let invalid_recipient = bech32::encode( + &format!("{}{}", PLUGIN_RECIPIENT_PREFIX, INVALID_PLUGIN_NAME), + [], + bech32::Variant::Bech32, + ) + .unwrap(); + assert!(invalid_recipient.parse::().is_err()); + } + + #[test] + fn identity_rejects_invalid_chars() { + let invalid_identity = bech32::encode( + &format!("{}{}-", PLUGIN_IDENTITY_PREFIX, INVALID_PLUGIN_NAME), + [], + bech32::Variant::Bech32, + ) + .expect("HRP is valid") + .to_uppercase(); + assert!(invalid_identity.parse::().is_err()); + } + + #[test] + #[should_panic] + fn identity_default_for_plugin_rejects_invalid_chars() { + Identity::default_for_plugin(INVALID_PLUGIN_NAME); + } + + #[test] + fn recipient_plugin_v1_rejects_invalid_chars() { + assert!(matches!( + RecipientPluginV1::new(INVALID_PLUGIN_NAME, &[], &[], NoCallbacks), + Err(EncryptError::MissingPlugin { binary_name }) if binary_name == INVALID_PLUGIN_NAME, + )); + } + + #[test] + fn identity_plugin_v1_rejects_invalid_chars() { + assert!(matches!( + IdentityPluginV1::new(INVALID_PLUGIN_NAME, &[], NoCallbacks), + Err(DecryptError::MissingPlugin { binary_name }) if binary_name == INVALID_PLUGIN_NAME, + )); + } } diff --git a/rage/CHANGELOG.md b/rage/CHANGELOG.md index 347ac10d..a56681c1 100644 --- a/rage/CHANGELOG.md +++ b/rage/CHANGELOG.md @@ -10,6 +10,13 @@ to 1.0.0 are beta releases. ## [Unreleased] +## [0.6.1, 0.7.2, 0.8.2, 0.9.3, 0.10.1, 0.11.1] - 2024-12-18 +### Security +- Fixed a security vulnerability that could allow an attacker to execute an + arbitrary binary under certain conditions. See GHSA-4fg7-vxc8-qx5w. Plugin + names are now required to only contain alphanumeric characters or the four + special characters `+-._`. Thanks to ⬡-49016 for reporting this issue. + ## [0.11.0] - 2024-11-03 ### Added - Partial French translation! diff --git a/rage/Cargo.toml b/rage/Cargo.toml index 799c03f4..a472f7fc 100644 --- a/rage/Cargo.toml +++ b/rage/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "rage" description = "[BETA] A simple, secure, and modern encryption tool." -version = "0.11.0" +version = "0.11.1" authors.workspace = true repository.workspace = true readme = "../README.md" diff --git a/rage/src/bin/rage/main.rs b/rage/src/bin/rage/main.rs index 8644ab4e..a0e5231d 100644 --- a/rage/src/bin/rage/main.rs +++ b/rage/src/bin/rage/main.rs @@ -234,6 +234,13 @@ fn write_output( Ok(()) } +#[inline] +fn valid_plugin_name(plugin_name: &str) -> bool { + plugin_name + .bytes() + .all(|b| b.is_ascii_alphanumeric() | matches!(b, b'+' | b'-' | b'.' | b'_')) +} + fn decrypt(opts: AgeOptions) -> Result<(), error::DecryptError> { if opts.armor { return Err(error::DecryptError::ArmorFlag); @@ -270,6 +277,12 @@ fn decrypt(opts: AgeOptions) -> Result<(), error::DecryptError> { let identities = if plugin_name.is_empty() { read_identities(opts.identity, opts.max_work_factor, &mut stdin_guard)? } else { + if !valid_plugin_name(plugin_name) { + return Err(age::DecryptError::MissingPlugin { + binary_name: plugin_name.into(), + } + .into()); + } // Construct the default plugin. vec![Box::new(plugin::IdentityPluginV1::new( plugin_name, diff --git a/rage/tests/cmd/rage-keygen/version.toml b/rage/tests/cmd/rage-keygen/version.toml index d3dad014..592c1fbe 100644 --- a/rage/tests/cmd/rage-keygen/version.toml +++ b/rage/tests/cmd/rage-keygen/version.toml @@ -1,6 +1,6 @@ bin.name = "rage-keygen" args = "--version" stdout = """ -rage-keygen 0.11.0 +rage-keygen 0.11.1 """ stderr = "" diff --git a/rage/tests/cmd/rage-mount/version.toml b/rage/tests/cmd/rage-mount/version.toml index 0117dd44..e30e8af4 100644 --- a/rage/tests/cmd/rage-mount/version.toml +++ b/rage/tests/cmd/rage-mount/version.toml @@ -1,6 +1,6 @@ bin.name = "rage-mount" args = "--version" stdout = """ -rage-mount 0.11.0 +rage-mount 0.11.1 """ stderr = "" diff --git a/rage/tests/cmd/rage/decrypt-invalid-identity-chars.in/file.age.txt b/rage/tests/cmd/rage/decrypt-invalid-identity-chars.in/file.age.txt new file mode 100644 index 00000000..6f9dc67f --- /dev/null +++ b/rage/tests/cmd/rage/decrypt-invalid-identity-chars.in/file.age.txt @@ -0,0 +1,8 @@ +-----BEGIN AGE ENCRYPTED FILE----- +YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBHUGc3Zlhpekp0K012aXdu +T1VZN0lmWlRmNjdLYVB4RldkTFVLTkNDUXlBCmJjRUcrM3E0a0U0N3IyK1JsTitG +dHVTd0N6TVFRTWgzdG5uSzJmNm9YMTgKLT4gQXQ1WWAtZ3JlYXNlIDxodGFSVHJg +IFg0cWYsO0ogZ2Fzc1EKZGtPSTB3Ci0tLSBKazRIaHJxdnNJcHpyclRkQjg3QW5r +SVE2MHdtWkErYTNrNWJibWd1bmNBCkK9FoOkiLB93gD79vNed8L3LM9rhKm5qma2 +lSiwRx/aM1DKaZO0CMmYQkoM2tPReA== +-----END AGE ENCRYPTED FILE----- diff --git a/rage/tests/cmd/rage/decrypt-invalid-identity-chars.toml b/rage/tests/cmd/rage/decrypt-invalid-identity-chars.toml new file mode 100644 index 00000000..d3350d2a --- /dev/null +++ b/rage/tests/cmd/rage/decrypt-invalid-identity-chars.toml @@ -0,0 +1,13 @@ +bin.name = "rage" +args = "--decrypt --identity - file.age.txt" +status = "failed" +stdin = """ +AGE-PLUGIN-FOOBAR/../../../../../../../USR/BIN/ECHO-1HKGPY3 +""" +stdout = "" +stderr = """ +Error: identity file contains non-identity data on line 1 + +[ Did rage not do what you expected? Could an error be more useful? ] +[ Tell us: https://str4d.xyz/rage/report ] +""" diff --git a/rage/tests/cmd/rage/decrypt-invalid-plugin-name-chars.toml b/rage/tests/cmd/rage/decrypt-invalid-plugin-name-chars.toml new file mode 100644 index 00000000..968a8b25 --- /dev/null +++ b/rage/tests/cmd/rage/decrypt-invalid-plugin-name-chars.toml @@ -0,0 +1,12 @@ +bin.name = "rage" +args = "--decrypt -j foobar/../../../../../../../usr/bin/echo" +status = "failed" +stdin = "" +stdout = "" +stderr = """ +Error: Could not find 'foobar/../../../../../../../usr/bin/echo' on the PATH. +Have you installed the plugin? + +[ Did rage not do what you expected? Could an error be more useful? ] +[ Tell us: https://str4d.xyz/rage/report ] +""" diff --git a/rage/tests/cmd/rage/encrypt-invalid-recipient-chars.toml b/rage/tests/cmd/rage/encrypt-invalid-recipient-chars.toml new file mode 100644 index 00000000..ce03d5e5 --- /dev/null +++ b/rage/tests/cmd/rage/encrypt-invalid-recipient-chars.toml @@ -0,0 +1,11 @@ +bin.name = "rage" +args = "--encrypt --recipient age1foobar/../../../../../../../usr/bin/echo1849l6e" +status = "failed" +stdin = "" +stdout = "" +stderr = """ +Error: Invalid recipient 'age1foobar/../../../../../../../usr/bin/echo1849l6e'. + +[ Did rage not do what you expected? Could an error be more useful? ] +[ Tell us: https://str4d.xyz/rage/report ] +""" diff --git a/rage/tests/cmd/rage/version.toml b/rage/tests/cmd/rage/version.toml index 3443bd2d..1f92da80 100644 --- a/rage/tests/cmd/rage/version.toml +++ b/rage/tests/cmd/rage/version.toml @@ -1,6 +1,6 @@ bin.name = "rage" args = "--version" stdout = """ -rage 0.11.0 +rage 0.11.1 """ stderr = ""