Skip to content

Commit

Permalink
AVRO-3225: AVRO-3226: Fix possible StackOverflowException and OutOfMe…
Browse files Browse the repository at this point in the history 
…moryException on invalid input

* AVRO-3225: Fix possible StackOverflowException on invalid input

* AVRO-3226: Fix possible OutOfMemoryException on invalid input

* AVRO-3226: Backport changes for netstandard2.0

(cherry picked from commit a1fce29)
Signed-off-by: Ryan Skraba <ryan@skraba.com>
  • Loading branch information
@PSanetra @RyanSkraba
PSanetra authored and RyanSkraba committed Oct 20, 2021
1 parent 423f40f commit 4e1fefc
Show file tree
Hide file tree
Showing 3 changed files with 163 additions and 21 deletions.
33 changes: 29 additions & 4 deletions lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
* limitations under the License.
*/
using System;
using System.IO;
using System.Text;

namespace Avro.IO
{
Expand All @@ -24,6 +26,11 @@ namespace Avro.IO
/// </content>
public partial class BinaryDecoder
{
/// <summary>
/// It is hard to find documentation about the real maximum array length in .NET Framework 4.6.1, but this seems to work :-/
/// </summary>
private const int MaxDotNetArrayLength = 0x3FFFFFFF;

/// <summary>
/// A float is written as 4 bytes.
/// The float is converted into a 32-bit integer using a method equivalent to
Expand Down Expand Up @@ -72,10 +79,28 @@ public double ReadDouble()
public string ReadString()
{
int length = ReadInt();
byte[] buffer = new byte[length];
//TODO: Fix this because it's lame;
ReadFixed(buffer);
return System.Text.Encoding.UTF8.GetString(buffer);

if (length < 0)
{
throw new AvroException("Can not deserialize a string with negative length!");
}

if (length > MaxDotNetArrayLength)
{
throw new AvroException("String length is not supported!");
}

using (var binaryReader = new BinaryReader(stream, Encoding.UTF8, true))
{
var bytes = binaryReader.ReadBytes(length);

if (bytes.Length != length)
{
throw new AvroException("Could not read as many bytes from stream as expected!");
}

return Encoding.UTF8.GetString(bytes);
}
}

private void Read(byte[] buffer, int start, int len)
Expand Down
56 changes: 45 additions & 11 deletions lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System;
using System.Buffers;
using System.Buffers.Binary;
using System.IO;
using System.Text;

namespace Avro.IO
Expand All @@ -28,6 +29,8 @@ namespace Avro.IO
public partial class BinaryDecoder
{
private const int StackallocThreshold = 256;
private const int MaxFastReadLength = 4096;
private const int MaxDotNetArrayLength = 0x7FFFFFC7;

/// <summary>
/// A float is written as 4 bytes.
Expand Down Expand Up @@ -63,23 +66,54 @@ public double ReadDouble()
/// <returns>String read from the stream.</returns>
public string ReadString()
{
byte[] bufferArray = null;

int length = ReadInt();
Span<byte> buffer = length <= StackallocThreshold ?
stackalloc byte[length] :
(bufferArray = ArrayPool<byte>.Shared.Rent(length)).AsSpan(0, length);

Read(buffer);

string result = Encoding.UTF8.GetString(buffer);
if (length < 0)
{
throw new AvroException("Can not deserialize a string with negative length!");
}

if (bufferArray != null)
if (length <= MaxFastReadLength)
{
ArrayPool<byte>.Shared.Return(bufferArray);
byte[] bufferArray = null;

try
{
Span<byte> buffer = length <= StackallocThreshold ?
stackalloc byte[length] :
(bufferArray = ArrayPool<byte>.Shared.Rent(length)).AsSpan(0, length);

Read(buffer);

return Encoding.UTF8.GetString(buffer);
}
finally
{
if (bufferArray != null)
{
ArrayPool<byte>.Shared.Return(bufferArray);
}
}
}
else
{
if (length > MaxDotNetArrayLength)
{
throw new AvroException("String length is not supported!");
}

return result;
using (var binaryReader = new BinaryReader(stream, Encoding.UTF8, true))
{
var bytes = binaryReader.ReadBytes(length);

if (bytes.Length != length)
{
throw new AvroException("Could not read as many bytes from stream as expected!");
}

return Encoding.UTF8.GetString(bytes);
}
}
}

private void Read(byte[] buffer, int start, int len)
Expand Down
95 changes: 89 additions & 6 deletions lang/csharp/src/apache/test/IO/BinaryCodecTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
using NUnit.Framework;
using System.IO;
using System.Linq;
using System.Text;
using Avro.IO;

namespace Avro.Test
Expand Down Expand Up @@ -214,23 +215,105 @@ public void TestString(string n, int overhead)
TestSkip(n, (Decoder d) => d.SkipString(), (Encoder e, string t) => e.WriteString(t), overhead + n.Length);
}

#if NETCOREAPP3_1
#if NETCOREAPP3_1_OR_GREATER
[Test]
public void TestLargeString()
public void TestStringReadIntoArrayPool()
{
const int maxFastReadLength = 4096;

// Create a 16KB buffer in the Array Pool
var largeBufferToSeedPool = ArrayPool<byte>.Shared.Rent(2 << 14);
ArrayPool<byte>.Shared.Return(largeBufferToSeedPool);

// Create a slightly less than 16KB buffer, which will use the 16KB buffer in the pool
var n = string.Concat(Enumerable.Repeat("1234567890", 1600));
var overhead = 3;
var n = string.Concat(Enumerable.Repeat("A", maxFastReadLength));
var overhead = 2;

TestRead(n, (Decoder d) => d.ReadString(), (Encoder e, string t) => e.WriteString(t), overhead + n.Length);
TestSkip(n, (Decoder d) => d.SkipString(), (Encoder e, string t) => e.WriteString(t), overhead + n.Length);
}

[Test]
public void TestStringReadByBinaryReader()
{
const int overhead = 2;
const int maxFastReadLength = 4096;
const int expectedStringLength = maxFastReadLength + 1;
var n = string.Concat(Enumerable.Repeat("A", expectedStringLength));

TestRead(n, (Decoder d) => d.ReadString(), (Encoder e, string t) => e.WriteString(t), expectedStringLength + overhead);
}
#endif

[Test]
public void TestInvalidInputWithNegativeStringLength()
{
using (MemoryStream iostr = new MemoryStream())
{
Encoder e = new BinaryEncoder(iostr);

e.WriteLong(-1);

iostr.Flush();
iostr.Position = 0;
Decoder d = new BinaryDecoder(iostr);

var exception = Assert.Throws<AvroException>(() => d.ReadString());

Assert.NotNull(exception);
Assert.AreEqual("Can not deserialize a string with negative length!", exception.Message);
iostr.Close();
}
}

[Test]
public void TestInvalidInputWithMaxIntAsStringLength()
{
using (MemoryStream iostr = new MemoryStream())
{
Encoder e = new BinaryEncoder(iostr);

e.WriteLong(int.MaxValue);
e.WriteBytes(Encoding.UTF8.GetBytes("SomeSmallString"));

iostr.Flush();
iostr.Position = 0;
Decoder d = new BinaryDecoder(iostr);

var exception = Assert.Throws<AvroException>(() => d.ReadString());

Assert.NotNull(exception);
Assert.AreEqual("String length is not supported!", exception.Message);
iostr.Close();
}
}

[Test]
public void TestInvalidInputWithMaxArrayLengthAsStringLength()
{
using (MemoryStream iostr = new MemoryStream())
{
Encoder e = new BinaryEncoder(iostr);

#if NETCOREAPP3_1_OR_GREATER
const int maximumArrayLength = 0x7FFFFFC7;
#else
const int maximumArrayLength = 0x7FFFFFFF / 2;
#endif

e.WriteLong(maximumArrayLength);
e.WriteBytes(Encoding.UTF8.GetBytes("SomeSmallString"));

iostr.Flush();
iostr.Position = 0;
Decoder d = new BinaryDecoder(iostr);

var exception = Assert.Throws<AvroException>(() => d.ReadString());

Assert.NotNull(exception);
Assert.AreEqual("Could not read as many bytes from stream as expected!", exception.Message);
iostr.Close();
}
}

[TestCase(0, 1)]
[TestCase(1, 1)]
[TestCase(64, 2)]
Expand Down

0 comments on commit 4e1fefc

Please sign in to comment.