Merge pull request #554 from str4d/bugfix-0.11.1

plugin: restrict characters in plugin names

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:

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

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"

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<C: Callbacks> RecipientPluginV1<C> {
         identities: &[Identity],
         callbacks: C,
     ) -> Result<Self, EncryptError> {
-        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<C: Callbacks> IdentityPluginV1<C> {
         identities: &[Identity],
         callbacks: C,
     ) -> Result<Self, DecryptError> {
-        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<Identity>, callbacks: C) -> Self {
@@ -697,7 +736,14 @@ impl<C: Callbacks> crate::Identity for IdentityPluginV1<C> {
 
 #[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::<Recipient>().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::<Identity>().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,
+        ));
+    }
 }

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!

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"

rage/src/bin/rage/main.rs

@@ -234,6 +234,13 @@ fn write_output<R: io::Read, W: io::Write>(
     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,

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 = ""

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 = ""

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-----

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                            ]
+"""

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                            ]
+"""