diff --git a/Age.Tests/PluginTests.cs b/Age.Tests/PluginTests.cs index a76b3aa..f6cc05e 100644 --- a/Age.Tests/PluginTests.cs +++ b/Age.Tests/PluginTests.cs @@ -107,6 +107,92 @@ public void PluginIdentity_ToString_ReturnsOriginalString() Assert.Equal(str, identity.ToString()); } + // --- Plugin name validation (security: prevents arbitrary executable paths) --- + + [Theory] + [InlineData("pwn/pwn")] // forward slash -> relative path to Process.Start + [InlineData("pwn\\pwn")] // backslash (Windows separator) + [InlineData("a b")] // space + public void PluginRecipient_ExtractPluginName_InvalidChars_Throws(string name) + { + // Valid bech32 (valid checksum), but the name carries a disallowed character. + var malicious = MakePluginRecipient(name); + var ex = Assert.Throws(() => PluginRecipient.ExtractPluginName(malicious)); + Assert.Contains("invalid plugin name", ex.Message); + } + + [Theory] + [InlineData("pwn/pwn")] + [InlineData("pwn\\pwn")] + public void PluginIdentity_ExtractPluginName_InvalidChars_Throws(string name) + { + var malicious = MakePluginIdentity(name); + var ex = Assert.Throws(() => PluginIdentity.ExtractPluginName(malicious)); + Assert.Contains("invalid plugin name", ex.Message); + } + + [Fact] + public void PluginRecipient_Constructor_PathSeparatorName_Throws() + { + // The separator must be rejected at construction, before any Process.Start can run. + Assert.Throws(() => new PluginRecipient(MakePluginRecipient("pwn/pwn"))); + } + + [Theory] + [InlineData("yubikey")] + [InlineData("fido2hmac")] + [InlineData("se-cure_plugin.v2")] // hyphen, underscore, dot, digit are all allowed + public void ExtractPluginName_ValidNames_Accepted(string name) + { + Assert.Equal(name, PluginRecipient.ExtractPluginName(MakePluginRecipient(name))); + } + + [Fact] + public void PluginConnection_InvalidName_RefusesToLaunch() + { + // Defense in depth at the process sink itself. + var ex = Assert.Throws(() => new PluginConnection("pwn/pwn", "recipient-v1")); + Assert.Contains("invalid name", ex.Message); + } + + [Theory] + [InlineData("")] // empty + [InlineData("pwn/pwn")] // forward slash + [InlineData("pwn\\pwn")] // backslash + [InlineData("a b")] // space + [InlineData("a\tb")] // tab + public void PluginNameValidator_RejectsInvalid(string name) + { + Assert.False(PluginNameValidator.IsValid(name)); + } + + [Theory] + [InlineData("yubikey")] + [InlineData("fido2hmac")] + [InlineData("se-cure_plugin.v2")] + public void PluginNameValidator_AcceptsValid(string name) + { + Assert.True(PluginNameValidator.IsValid(name)); + } + + [Fact] + public void PluginRecipient_ExtractPluginName_DegenerateHrp_ThrowsFormatException() + { + // HRP decodes to just "age" (no name) — must be a clean FormatException, + // not the ArgumentOutOfRangeException that hrp[4..] would have thrown. + var s = Bech32.Encode("age", new byte[] { 0x01, 0x02, 0x03 }); + Assert.Throws(() => PluginRecipient.ExtractPluginName(s)); + } + + [Fact] + public void PluginIdentity_ExtractPluginName_DegenerateHrp_ThrowsFormatException() + { + // HRP decodes to exactly "age-plugin-" (no name) — must be a clean FormatException, + // not the ArgumentOutOfRangeException that hrp[11..^1] would have thrown. + var s = Bech32.Encode("age-plugin-", new byte[] { 0x01, 0x02, 0x03 }); + Assert.Throws(() => PluginIdentity.ExtractPluginName(s)); + } + // --- PluginConnection stanza I/O roundtrip --- [Fact] diff --git a/Age/Plugin/PluginConnection.cs b/Age/Plugin/PluginConnection.cs index 6ed6545..b1f4fad 100644 --- a/Age/Plugin/PluginConnection.cs +++ b/Age/Plugin/PluginConnection.cs @@ -15,6 +15,11 @@ internal sealed class PluginConnection : IDisposable /// public PluginConnection(string pluginName, string stateMachine) { + // Defense in depth: the caller should already have validated the name, but this is + // the sink that turns it into an executable path, so never launch an invalid one. + if (!PluginNameValidator.IsValid(pluginName)) + throw new AgePluginException($"refusing to launch plugin with invalid name: '{pluginName}'"); + var binaryName = $"age-plugin-{pluginName}"; var startInfo = new ProcessStartInfo { diff --git a/Age/Plugin/PluginNameValidator.cs b/Age/Plugin/PluginNameValidator.cs new file mode 100644 index 0000000..d3f2b34 --- /dev/null +++ b/Age/Plugin/PluginNameValidator.cs @@ -0,0 +1,34 @@ +namespace Age.Plugin; + +/// +/// Validation for age plugin names. A plugin name becomes part of the +/// age-plugin-<name> executable that is launched via Process.Start, +/// so it must never contain a path separator (or any other character that could turn +/// that filename into a relative or absolute path). The allowed set mirrors the +/// reference implementation's validPluginName. +/// +internal static class PluginNameValidator +{ + private static bool IsAllowed(char c) => + char.IsAsciiLetterOrDigit(c) || c is '+' or '-' or '.' or '_'; + + public static bool IsValid(string name) + { + if (name.Length == 0) + return false; + + foreach (var c in name) + { + if (!IsAllowed(c)) + return false; + } + + return true; + } + + /// Returns if valid, otherwise throws . + public static string Validate(string name) => + IsValid(name) + ? name + : throw new FormatException($"invalid plugin name: '{name}'"); +} diff --git a/Age/Recipients/PluginIdentity.cs b/Age/Recipients/PluginIdentity.cs index 57e6caa..e81a6c9 100644 --- a/Age/Recipients/PluginIdentity.cs +++ b/Age/Recipients/PluginIdentity.cs @@ -141,10 +141,15 @@ internal static string ExtractPluginName(string identity) // Bech32-decode to get HRP. For "AGE-PLUGIN-YUBIKEY-1...", HRP = "age-plugin-yubikey-", name = HRP[11..^1] = "yubikey" var (hrp, _) = Bech32.Decode(identity); - // skip "age-plugin-" prefix and trailing "-" - return hrp.StartsWith("age-plugin-") + // A plugin identity HRP is "age-plugin--"; require both affixes (with room + // between them) so the hrp[11..^1] slice can't run out of range on a malformed value. + var name = hrp.StartsWith("age-plugin-") && hrp.EndsWith("-") && hrp.Length > 11 ? hrp[11..^1] : throw new FormatException($"invalid plugin identity HRP: {hrp}"); + + // The name becomes the age-plugin- executable path, so reject anything + // outside the allowed set (notably path separators) before it reaches Process.Start. + return PluginNameValidator.Validate(name); } public override string ToString() => diff --git a/Age/Recipients/PluginRecipient.cs b/Age/Recipients/PluginRecipient.cs index 7e86d08..af5bac5 100644 --- a/Age/Recipients/PluginRecipient.cs +++ b/Age/Recipients/PluginRecipient.cs @@ -135,10 +135,15 @@ internal static string ExtractPluginName(string recipient) // Bech32-decode to get HRP. For "age1yubikey1...", HRP = "age1yubikey", name = HRP[4..] = "yubikey" var (hrp, _) = Bech32.Decode(recipient); - // skip "age" + the "1" separator character encoded in hrp after "age" - return hrp.StartsWith("age") + // A plugin recipient HRP is "age1"; require the "age1" prefix so hrp[4..] + // is always in range (a shorter HRP like "age" would otherwise throw). + var name = hrp.StartsWith("age1") ? hrp[4..] : throw new FormatException($"invalid plugin recipient HRP: {hrp}"); + + // The name becomes the age-plugin- executable path, so reject anything + // outside the allowed set (notably path separators) before it reaches Process.Start. + return PluginNameValidator.Validate(name); } public override string ToString() =>