You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@avro.apache.org by rs...@apache.org on 2021/10/20 10:46:15 UTC
[avro] branch master updated: AVRO-3225: AVRO-3226: Fix possible
StackOverflowException and OutOfMemoryException on invalid input
This is an automated email from the ASF dual-hosted git repository.
rskraba pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/avro.git
The following commit(s) were added to refs/heads/master by this push:
new a1fce29 AVRO-3225: AVRO-3226: Fix possible StackOverflowException and OutOfMemoryException on invalid input
a1fce29 is described below
commit a1fce29d9675b4dd95dfee9db32cc505d0b2227c
Author: Philip Sanetra <PS...@users.noreply.github.com>
AuthorDate: Wed Oct 20 12:46:10 2021 +0200
AVRO-3225: AVRO-3226: Fix possible StackOverflowException and OutOfMemoryException 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
---
.../apache/main/IO/BinaryDecoder.netstandard2.0.cs | 33 +++++++-
.../main/IO/BinaryDecoder.notnetstandard2.0.cs | 56 ++++++++++---
lang/csharp/src/apache/test/IO/BinaryCodecTests.cs | 95 ++++++++++++++++++++--
3 files changed, 163 insertions(+), 21 deletions(-)
diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs
index 91afeb5..8c6cb7e 100644
--- a/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs
+++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs
@@ -16,6 +16,8 @@
* limitations under the License.
*/
using System;
+using System.IO;
+using System.Text;
namespace Avro.IO
{
@@ -25,6 +27,11 @@ namespace Avro.IO
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
/// Java's floatToIntBits and then encoded in little-endian format.
@@ -72,10 +79,28 @@ namespace Avro.IO
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)
diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs
index 17bd841..a3bd217 100644
--- a/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs
+++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs
@@ -18,6 +18,7 @@
using System;
using System.Buffers;
using System.Buffers.Binary;
+using System.IO;
using System.Text;
namespace Avro.IO
@@ -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.
@@ -63,23 +66,54 @@ namespace Avro.IO
/// <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)
diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs
index a6a1731..f894d7b 100644
--- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs
+++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs
@@ -20,6 +20,7 @@ using System.Buffers;
using NUnit.Framework;
using System.IO;
using System.Linq;
+using System.Text;
using Avro.IO;
namespace Avro.Test
@@ -214,23 +215,105 @@ namespace Avro.Test
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)]