Merge pull request #554 from str4d/bugfix-0.11.1

plugin: restrict characters in plugin names
This commit is contained in:
Jack Grigg 2024-12-19 04:46:19 +13:00 committed by GitHub
commit d7c727aef9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
15 changed files with 216 additions and 54 deletions

4
Cargo.lock generated
View file

@ -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",

View file

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

View file

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

View file

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

View file

@ -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,
));
}
}

View file

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

View file

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

View file

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

View file

@ -1,6 +1,6 @@
bin.name = "rage-keygen"
args = "--version"
stdout = """
rage-keygen 0.11.0
rage-keygen 0.11.1
"""
stderr = ""

View file

@ -1,6 +1,6 @@
bin.name = "rage-mount"
args = "--version"
stdout = """
rage-mount 0.11.0
rage-mount 0.11.1
"""
stderr = ""

View file

@ -0,0 +1,8 @@
-----BEGIN AGE ENCRYPTED FILE-----
YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBHUGc3Zlhpekp0K012aXdu
T1VZN0lmWlRmNjdLYVB4RldkTFVLTkNDUXlBCmJjRUcrM3E0a0U0N3IyK1JsTitG
dHVTd0N6TVFRTWgzdG5uSzJmNm9YMTgKLT4gQXQ1WWAtZ3JlYXNlIDxodGFSVHJg
IFg0cWYsO0ogZ2Fzc1EKZGtPSTB3Ci0tLSBKazRIaHJxdnNJcHpyclRkQjg3QW5r
SVE2MHdtWkErYTNrNWJibWd1bmNBCkK9FoOkiLB93gD79vNed8L3LM9rhKm5qma2
lSiwRx/aM1DKaZO0CMmYQkoM2tPReA==
-----END AGE ENCRYPTED FILE-----

View file

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

View file

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

View file

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

View file

@ -1,6 +1,6 @@
bin.name = "rage"
args = "--version"
stdout = """
rage 0.11.0
rage 0.11.1
"""
stderr = ""