Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

plugin: restrict characters in plugin names #554

Merged
merged 16 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
7 changes: 7 additions & 0 deletions age/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion age/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
183 changes: 137 additions & 46 deletions age/src/plugin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@
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)
}
Expand Down Expand Up @@ -104,10 +111,15 @@
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(),

Check warning on line 118 in age/src/plugin.rs

View check run for this annotation

Codecov / codecov/patch

age/src/plugin.rs#L116-L118

Added lines #L116 - L118 were not covered by tests
})
} else {
Err("invalid plugin name")
}
} else {
Err("invalid HRP")
}
Expand Down Expand Up @@ -148,14 +160,20 @@
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")
}
Expand All @@ -171,16 +189,25 @@

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.
Expand Down Expand Up @@ -359,22 +386,28 @@
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,

Check warning on line 404 in age/src/plugin.rs

View check run for this annotation

Codecov / codecov/patch

age/src/plugin.rs#L390-L404

Added lines #L390 - L404 were not covered by tests
})
} else {
Err(EncryptError::MissingPlugin {
binary_name: plugin_name.to_string(),
})
}
}
}

Expand Down Expand Up @@ -563,16 +596,22 @@
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)

Check warning on line 608 in age/src/plugin.rs

View check run for this annotation

Codecov / codecov/patch

age/src/plugin.rs#L600-L608

Added lines #L600 - L608 were not covered by tests
})
} else {
Err(DecryptError::MissingPlugin {
binary_name: plugin_name.to_string(),
})
}
}

pub(crate) fn from_parts(plugin: Plugin, identities: Vec<Identity>, callbacks: C) -> Self {
Expand Down Expand Up @@ -697,7 +736,14 @@

#[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() {
Expand All @@ -706,4 +752,49 @@
"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,
));
}
}
7 changes: 7 additions & 0 deletions rage/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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!
Expand Down
2 changes: 1 addition & 1 deletion rage/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
13 changes: 13 additions & 0 deletions rage/src/bin/rage/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion rage/tests/cmd/rage-keygen/version.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
bin.name = "rage-keygen"
args = "--version"
stdout = """
rage-keygen 0.11.0
rage-keygen 0.11.1
"""
stderr = ""
2 changes: 1 addition & 1 deletion rage/tests/cmd/rage-mount/version.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
bin.name = "rage-mount"
args = "--version"
stdout = """
rage-mount 0.11.0
rage-mount 0.11.1
"""
stderr = ""
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-----BEGIN AGE ENCRYPTED FILE-----
YWdlLWVuY3J5cHRpb24ub3JnL3YxCi0+IFgyNTUxOSBHUGc3Zlhpekp0K012aXdu
T1VZN0lmWlRmNjdLYVB4RldkTFVLTkNDUXlBCmJjRUcrM3E0a0U0N3IyK1JsTitG
dHVTd0N6TVFRTWgzdG5uSzJmNm9YMTgKLT4gQXQ1WWAtZ3JlYXNlIDxodGFSVHJg
IFg0cWYsO0ogZ2Fzc1EKZGtPSTB3Ci0tLSBKazRIaHJxdnNJcHpyclRkQjg3QW5r
SVE2MHdtWkErYTNrNWJibWd1bmNBCkK9FoOkiLB93gD79vNed8L3LM9rhKm5qma2
lSiwRx/aM1DKaZO0CMmYQkoM2tPReA==
-----END AGE ENCRYPTED FILE-----
13 changes: 13 additions & 0 deletions rage/tests/cmd/rage/decrypt-invalid-identity-chars.toml
Original file line number Diff line number Diff line change
@@ -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 ]
"""
12 changes: 12 additions & 0 deletions rage/tests/cmd/rage/decrypt-invalid-plugin-name-chars.toml
Original file line number Diff line number Diff line change
@@ -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 ]
"""
Loading
Loading