diff --git a/age/CHANGELOG.md b/age/CHANGELOG.md index 2543e30f..2c22bea2 100644 --- a/age/CHANGELOG.md +++ b/age/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to Rust's notion of to 1.0.0 are beta releases. ## [Unreleased] +### 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.6.0] - 2021-05-02 ### Security diff --git a/age/src/plugin.rs b/age/src/plugin.rs index 387bf9ff..494f593f 100644 --- a/age/src/plugin.rs +++ b/age/src/plugin.rs @@ -29,6 +29,13 @@ const CMD_REQUEST_PUBLIC: &str = "request-public"; const CMD_REQUEST_SECRET: &str = "request-secret"; const CMD_FILE_KEY: &str = "file-key"; +#[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) } @@ -53,10 +60,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") } @@ -97,14 +109,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") } @@ -178,22 +196,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(), }) + } } } @@ -344,17 +368,23 @@ impl IdentityPluginV1 { identities: &[Identity], callbacks: C, ) -> Result { - 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>( diff --git a/rage/CHANGELOG.md b/rage/CHANGELOG.md index 922666d3..17879dd1 100644 --- a/rage/CHANGELOG.md +++ b/rage/CHANGELOG.md @@ -9,6 +9,12 @@ and this project adheres to Rust's notion of to 1.0.0 are beta releases. ## [Unreleased] +### 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.6.0] - 2021-05-02 ### Added