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() {