Merge branch 'bugfix-0.9.3' into bugfix-0.10.1

age/CHANGELOG.md

@@ -10,6 +10,14 @@ to 1.0.0 are beta releases.
 
 ## [Unreleased]
 
+## [0.6.1, 0.7.2, 0.8.2, 0.9.3] - 2024-11-18
+### Security
+- The age plugin protocol previously allowed plugin names that could be
+  interpreted as file paths. Under certain conditions, this could lead to a
+  different binary being executed as an age plugin than intended. Plugin names
+  are now required to only contain alphanumeric characters or the four special
+  characters `+-._`.
+
 ## [0.10.0] - 2024-02-04
 ### Added
 - Russian translation!

age/src/plugin.rs

@@ -41,6 +41,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)
 }
@@ -102,10 +109,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")
                 }
@@ -146,14 +158,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")
                 }
@@ -169,16 +187,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.
@@ -357,22 +384,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(),
             })
+        }
     }
 }
 
@@ -527,17 +560,23 @@ 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| IdentityPluginV1 {
-                plugin,
-                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| DecryptError::MissingPlugin { binary_name })
+                .map(|plugin| IdentityPluginV1 {
+                    plugin,
+                    identities: identities
+                        .iter()
+                        .filter(|r| r.name == plugin_name)
+                        .cloned()
+                        .collect(),
+                    callbacks,
+                })
+        } else {
+            Err(DecryptError::MissingPlugin {
+                binary_name: plugin_name.to_string(),
             })
+        }
     }
 
     fn unwrap_stanzas<'a>(
@@ -651,7 +690,29 @@ impl<C: Callbacks> crate::Identity for IdentityPluginV1<C> {
 
 #[cfg(test)]
 mod tests {
-    use super::Identity;
+    use crate::{Callbacks, DecryptError, EncryptError};
+
+    use super::{
+        Identity, IdentityPluginV1, Recipient, RecipientPluginV1, PLUGIN_IDENTITY_PREFIX,
+        PLUGIN_RECIPIENT_PREFIX,
+    };
+
+    const INVALID_PLUGIN_NAME: &str = "foobar/../../../../../../../usr/bin/echo";
+
+    #[derive(Clone)]
+    struct NoCallbacks;
+    impl Callbacks for NoCallbacks {
+        fn display_message(&self, _: &str) {}
+        fn confirm(&self, _: &str, _: &str, _: Option<&str>) -> Option<bool> {
+            None
+        }
+        fn request_public_string(&self, _: &str) -> Option<String> {
+            None
+        }
+        fn request_passphrase(&self, _: &str) -> Option<crate::secrecy::SecretString> {
+            None
+        }
+    }
 
     #[test]
     fn default_for_plugin() {
@@ -660,4 +721,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,14 @@ to 1.0.0 are beta releases.
 
 ## [Unreleased]
 
+## [0.6.1, 0.7.2, 0.8.2, 0.9.3] - 2024-11-18
+### Security
+- The age plugin protocol previously allowed plugin names that could be
+  interpreted as file paths. Under certain conditions, this could lead to a
+  different binary being executed as an age plugin than intended. Plugin names
+  are now required to only contain alphanumeric characters or the four special
+  characters `+-._`.
+
 ## [0.10.0] - 2024-02-04
 ### Added
 - Russian translation!

rage/src/bin/rage/main.rs

@@ -227,6 +227,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);
@@ -263,6 +270,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/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                            ]
+"""

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