From 0d2717748cf39fec5f98fb9790e2f07334913a30 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 29 Oct 2026 23:31:12 +0000 Subject: [PATCH] Decode Hex and Base64 from a copy when the source overlaps the destination [patch] TryEncode copies an overlapping source to a pooled buffer first (#138), but TryDecode did not. With the destination starting after the source in the same buffer, Hex overwrote encoded pairs before reading them and Base64.DecodeFromUtf8, which does not support overlap, corrupted or rejected valid input. Both returned true with corrupted output in most cases, and the Hex and Base64 obfuscators' TryDeobfuscate forward to these decoders. TryDecode now does what TryEncode does: when the spans overlap, it decodes from a pooled copy and clears the copy before returning it. Decode-direction overlap tests sit next to the encode ones and cover every IEncodingProvider and IObfuscationProvider at offsets (0,0), (0,3) and (3,0). Fixes #166 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW6XNeWiDxG4BVzYTra --- .../Base64EncodingProvider.cs | 19 +++++++++++++-- .../HexEncodingProvider.cs | 19 +++++++++++++-- Essentials.Tests/EncodingProviderTests.cs | 22 ++++++++++++++++++ Essentials.Tests/ObfuscationProviderTests.cs | 23 +++++++++++++++++++ 4 files changed, 79 insertions(+), 4 deletions(-) diff --git a/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs b/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs index 0606db3..ec74849 100644 --- a/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs +++ b/Essentials.EncodingProviders.Base64/Base64EncodingProvider.cs @@ -15,8 +15,8 @@ namespace ktsu.Essentials.EncodingProviders.Base64; /// /// /// The span paths use the UTF8 Base64 primitives directly, so they neither allocate an intermediate -/// string nor copy through a temporary array, except that encoding copies the source to a pooled buffer -/// first when it overlaps the destination. +/// string nor copy through a temporary array, except that encoding and decoding copy the source to a pooled +/// buffer first when it overlaps the destination. /// public class Base64EncodingProvider : IEncodingProvider { @@ -110,6 +110,21 @@ public bool TryEncode(Stream data, Stream destination) /// public bool TryDecode(ReadOnlySpan encodedData, Span destination, out int bytesWritten) { + if (encodedData.Overlaps(destination)) + { + // Base64.DecodeFromUtf8 does not support overlapping buffers, so decode from a copy. + byte[] copy = ArrayPool.Shared.Rent(encodedData.Length); + try + { + encodedData.CopyTo(copy); + return TryDecode(copy.AsSpan(0, encodedData.Length), destination, out bytesWritten); + } + finally + { + ArrayPool.Shared.Return(copy, clearArray: true); + } + } + #if !NET8_0_OR_GREATER if (encodedData.IndexOfAny(Whitespace) >= 0) { diff --git a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs index 1997807..8428709 100644 --- a/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs +++ b/Essentials.EncodingProviders.Hex/HexEncodingProvider.cs @@ -14,8 +14,8 @@ namespace ktsu.Essentials.EncodingProviders.Hex; /// /// /// The span paths convert byte by byte, so they neither allocate an intermediate string nor copy -/// through a temporary array, except that encoding copies the source to a pooled buffer first when it -/// overlaps the destination. Encoding emits uppercase; decoding accepts either case. +/// through a temporary array, except that encoding and decoding copy the source to a pooled buffer first +/// when it overlaps the destination. Encoding emits uppercase; decoding accepts either case. /// public class HexEncodingProvider : IEncodingProvider { @@ -124,6 +124,21 @@ public bool TryDecode(ReadOnlySpan encodedData, Span destination, ou return false; } + if (encodedData.Overlaps(destination)) + { + // A destination starting after the source would overwrite pairs the loop has not read yet, so decode from a copy. + byte[] copy = ArrayPool.Shared.Rent(encodedData.Length); + try + { + encodedData.CopyTo(copy); + return TryDecode(copy.AsSpan(0, encodedData.Length), destination, out bytesWritten); + } + finally + { + ArrayPool.Shared.Return(copy, clearArray: true); + } + } + for (int i = 0; i < encodedData.Length; i += 2) { if (!TryParseNibble(encodedData[i], out int high) || !TryParseNibble(encodedData[i + 1], out int low)) diff --git a/Essentials.Tests/EncodingProviderTests.cs b/Essentials.Tests/EncodingProviderTests.cs index da07153..fba7074 100644 --- a/Essentials.Tests/EncodingProviderTests.cs +++ b/Essentials.Tests/EncodingProviderTests.cs @@ -51,6 +51,28 @@ public void Encoding_OverlappingBuffers_MatchesOutOfPlace(IEncodingProvider enco } } + [TestMethod] + [DynamicData(nameof(EncodingProviders))] + public void Decoding_OverlappingBuffers_MatchesOutOfPlace(IEncodingProvider encoder, string providerName) + { + byte[] original = Encoding.ASCII.GetBytes("0123456789ab"); + byte[] encoded = encoder.Encode(original); + int maxLength = encoder.GetMaxDecodedLength(encoded.Length); + + // A destination after the source used to overwrite encoded bytes before they were read (issue #166). + foreach ((int sourceOffset, int destinationOffset) in new[] { (0, 0), (0, 3), (3, 0) }) + { + byte[] buffer = new byte[Math.Max(sourceOffset + encoded.Length, destinationOffset + maxLength)]; + encoded.CopyTo(buffer, sourceOffset); + + Assert.IsTrue( + encoder.TryDecode(buffer.AsSpan(sourceOffset, encoded.Length), buffer.AsSpan(destinationOffset, maxLength), out int written), + $"{providerName} should decode with source at {sourceOffset} and destination at {destinationOffset}"); + CollectionAssert.AreEqual(original, buffer[destinationOffset..(destinationOffset + written)], + $"{providerName} should decode with source at {sourceOffset} and destination at {destinationOffset} as it does out of place"); + } + } + [TestMethod] [DynamicData(nameof(EncodingProviders))] public void GetMaxEncodedLength_Throws_When_The_Bound_Does_Not_Fit_An_Int(IEncodingProvider encoder, string providerName) diff --git a/Essentials.Tests/ObfuscationProviderTests.cs b/Essentials.Tests/ObfuscationProviderTests.cs index e0dc5de..8ee5aeb 100644 --- a/Essentials.Tests/ObfuscationProviderTests.cs +++ b/Essentials.Tests/ObfuscationProviderTests.cs @@ -141,6 +141,29 @@ public void Obfuscation_OverlappingBuffers_MatchesOutOfPlace(IObfuscationProvide } } + [TestMethod] + [DynamicData(nameof(ObfuscationProviders))] + public void Deobfuscation_OverlappingBuffers_MatchesOutOfPlace(IObfuscationProvider provider, string providerName) + { + byte[] original = Encoding.ASCII.GetBytes("0123456789ab"); + byte[] obfuscated = provider.Obfuscate(original); + int maxLength = provider.GetMaxDeobfuscatedLength(obfuscated.Length); + + // The decode direction of the test above: a destination after the source used to read bytes + // it had already overwritten (issue #166). + foreach ((int sourceOffset, int destinationOffset) in new[] { (0, 0), (0, 3), (3, 0) }) + { + byte[] buffer = new byte[System.Math.Max(sourceOffset + obfuscated.Length, destinationOffset + maxLength)]; + obfuscated.CopyTo(buffer, sourceOffset); + + Assert.IsTrue( + provider.TryDeobfuscate(buffer.AsSpan(sourceOffset, obfuscated.Length), buffer.AsSpan(destinationOffset, maxLength), out int written), + $"{providerName} should deobfuscate with source at {sourceOffset} and destination at {destinationOffset}"); + CollectionAssert.AreEqual(original, buffer[destinationOffset..(destinationOffset + written)], + $"{providerName} should deobfuscate with source at {sourceOffset} and destination at {destinationOffset} as it does out of place"); + } + } + [TestMethod] public void Obfuscation_Base64_Roundtrip_String() {