From 58ef260bc1cc7a98462249e2624087f2bf316f14 Mon Sep 17 00:00:00 2001 From: Patrick Scheid Date: Thu, 23 Jul 2026 02:48:21 +0200 Subject: [PATCH 1/3] Validate plugin names to prevent arbitrary executable paths (#11) A plugin recipient/identity carries the plugin name in its bech32 HRP. Bech32.Decode validates the checksum and data charset but not the HRP, so a name could contain a path separator, which then flowed into 'age-plugin-' and Process.Start. Because .NET treats a FileName containing a separator as a path relative to the working directory (rather than a PATH lookup), a crafted recipient/identity could execute an arbitrary binary (e.g. 'age1pwn/pwn1...' -> 'age-plugin-pwn/pwn'). - Add PluginNameValidator with an allowlist matching age's validPluginName ([A-Za-z0-9+-._]; no path separators). - Enforce it in ExtractPluginName for both PluginRecipient and PluginIdentity, so a bad name is rejected at construction, before any process launch. - Defense in depth: PluginConnection also refuses an invalid name at the Process.Start sink. - Add regression tests (path separators, backslash, space rejected; valid names still accepted). Fixes #11 --- Age.Tests/PluginTests.cs | 48 +++++++++++++++++++++++++++++++ Age/Plugin/PluginConnection.cs | 5 ++++ Age/Plugin/PluginNameValidator.cs | 34 ++++++++++++++++++++++ Age/Recipients/PluginIdentity.cs | 6 +++- Age/Recipients/PluginRecipient.cs | 6 +++- 5 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 Age/Plugin/PluginNameValidator.cs diff --git a/Age.Tests/PluginTests.cs b/Age.Tests/PluginTests.cs index a76b3aa..ebca369 100644 --- a/Age.Tests/PluginTests.cs +++ b/Age.Tests/PluginTests.cs @@ -107,6 +107,54 @@ 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); + } + // --- 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..8eadc6f 100644 --- a/Age/Recipients/PluginIdentity.cs +++ b/Age/Recipients/PluginIdentity.cs @@ -142,9 +142,13 @@ internal static string ExtractPluginName(string identity) var (hrp, _) = Bech32.Decode(identity); // skip "age-plugin-" prefix and trailing "-" - return hrp.StartsWith("age-plugin-") + var name = hrp.StartsWith("age-plugin-") ? 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..2dbc2f7 100644 --- a/Age/Recipients/PluginRecipient.cs +++ b/Age/Recipients/PluginRecipient.cs @@ -136,9 +136,13 @@ internal static string ExtractPluginName(string recipient) var (hrp, _) = Bech32.Decode(recipient); // skip "age" + the "1" separator character encoded in hrp after "age" - return hrp.StartsWith("age") + var name = hrp.StartsWith("age") ? 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() => From bb26c3d9e1d4ef4982a358999d60d4204dc1b423 Mon Sep 17 00:00:00 2001 From: Patrick Scheid Date: Thu, 23 Jul 2026 02:52:11 +0200 Subject: [PATCH 2/3] Add direct PluginNameValidator tests (cover empty-name branch) --- Age.Tests/PluginTests.cs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/Age.Tests/PluginTests.cs b/Age.Tests/PluginTests.cs index ebca369..143ef73 100644 --- a/Age.Tests/PluginTests.cs +++ b/Age.Tests/PluginTests.cs @@ -155,6 +155,26 @@ public void PluginConnection_InvalidName_RefusesToLaunch() 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)); + } + // --- PluginConnection stanza I/O roundtrip --- [Fact] From ee7da429644c556b4002d78c3d9f19d242ecdd6e Mon Sep 17 00:00:00 2001 From: Patrick Scheid Date: Thu, 23 Jul 2026 03:07:26 +0200 Subject: [PATCH 3/3] Harden plugin HRP parsing against out-of-range slices ExtractPluginName sliced the decoded HRP (hrp[4..] / hrp[11..^1]) after only a loose StartsWith check, so a degenerate HRP threw ArgumentOutOfRangeException instead of a clean FormatException: - recipient: an HRP of just "age" -> hrp[4..] - identity: an HRP of exactly "age-plugin-" -> hrp[11..^1] Require the full expected shape ('age1' / 'age-plugin--') before slicing, so malformed input yields FormatException. Add regression tests for both. --- Age.Tests/PluginTests.cs | 18 ++++++++++++++++++ Age/Recipients/PluginIdentity.cs | 5 +++-- Age/Recipients/PluginRecipient.cs | 5 +++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/Age.Tests/PluginTests.cs b/Age.Tests/PluginTests.cs index 143ef73..f6cc05e 100644 --- a/Age.Tests/PluginTests.cs +++ b/Age.Tests/PluginTests.cs @@ -175,6 +175,24 @@ 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/Recipients/PluginIdentity.cs b/Age/Recipients/PluginIdentity.cs index 8eadc6f..e81a6c9 100644 --- a/Age/Recipients/PluginIdentity.cs +++ b/Age/Recipients/PluginIdentity.cs @@ -141,8 +141,9 @@ 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 "-" - var name = 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}"); diff --git a/Age/Recipients/PluginRecipient.cs b/Age/Recipients/PluginRecipient.cs index 2dbc2f7..af5bac5 100644 --- a/Age/Recipients/PluginRecipient.cs +++ b/Age/Recipients/PluginRecipient.cs @@ -135,8 +135,9 @@ 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" - var name = 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}");