AVRO-3225 & AVRO-3226: Fix possible StackOverflowException and OutOfMemoryException on invalid input

[PSanetra]

Jira

• My PR addresses the following Avro Jira issues and references them in the PR title.
  • AVRO-3225
  • AVRO-3226

Tests

• My PR adds the following unit tests:
  • BinaryCodecTests.TestStringReadIntoArrayPool (replaces BinaryCodecTests.TestLargeString)
  • BinaryCodecTests.TestStringReadByBinaryReader
  • BinaryCodecTests.TestInvalidInputWithNegativeStringLength
  • BinaryCodecTests.TestInvalidInputWithMaxIntAsStringLength
  • BinaryCodecTests.TestInvalidInputWithMaxArrayLengthAsStringLength

Commits

• My commits all reference Jira issues in their subject lines. In addition, my commits follow the guidelines from "How to write a good git commit message":
  1. Subject is separated from body by a blank line
  2. Subject is limited to 50 characters (not including Jira issue reference)
  3. Subject does not end with a period
  4. Subject uses the imperative mood ("add", not "adding")
  5. Body wraps at 72 characters
  6. Body explains "what" and "why", not "how"

Documentation

• In case of new functionality, my PR adds documentation that describes how to use it.
  • All the public functions and the classes in the PR contain Javadoc that explain what it does

[iemejia]

R: @blachniet mind to take a look please
CC: @RyanSkraba this fix (when merged) should be cherry picked into 1.11.0

[blachniet]

This test is failing when running on .NET Framework 4.6.1. It looks like we aren't running the Windows builds/tests in GitHub actions anymore, which would explain why that didn't catch it.

+ dotnet test --configuration Release --no-build --filter 'TestCategory!=Interop' Avro.sln
Test run for C:\Users\Brian.Lachniet\code\github.com\apache\avro\lang\csharp\src\apache\test\bin\Release\net461\Avro.test.dll (.NETFramework,Version=v4.6.1)
Microsoft (R) Test Execution Command Line Tool Version 16.11.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
  Failed TestInvalidInputWithMaxArrayLengthAsStringLength [75 ms]
  Error Message:
     Expected: <Avro.AvroException>
  But was:  <System.OutOfMemoryException: Exception of type 'System.OutOfMemoryException' was thrown.
   at System.IO.BinaryReader.ReadBytes(Int32 count)
   at Avro.IO.BinaryDecoder.ReadString() in C:\Users\Brian.Lachniet\code\github.com\apache\avro\lang\csharp\src\apache\main\IO\BinaryDecoder.netstandard2.0.cs:line 90
   at Avro.Test.BinaryCodecTests.<>c__DisplayClass11_0.<TestInvalidInputWithMaxArrayLengthAsStringLength>b__0() in C:\Users\Brian.Lachniet\code\github.com\apache\avro\lang\csharp\src\apache\test\IO\BinaryCodecTests.cs:line 304
   at NUnit.Framework.Assert.Throws(IResolveConstraint expression, TestDelegate code, String message, Object[] args)>

  Stack Trace:
     at Avro.Test.BinaryCodecTests.TestInvalidInputWithMaxArrayLengthAsStringLength() in C:\Users\Brian.Lachniet\code\github.com\apache\avro\lang\csharp\src\apache\test\IO\BinaryCodecTests.cs:line 304


Failed!  - Failed:     1, Passed:   626, Skipped:     0, Total:   627, Duration: 3 s - Avro.test.dll (net461)
Test run for C:\Users\Brian.Lachniet\code\github.com\apache\avro\lang\csharp\src\apache\test\bin\Release\netcoreapp2.1\Avro.test.dll (.NETCoreApp,Version=v2.1)

The Remarks on the Array Class indicate that the .NET Framework may have a smaller size limit. However, even after setting MaxDotNetArrayLength = 0x77359400;, I'm getting the same test failure (only on the net461 tests).

@RyanSkraba , how does the Java implementation handle large strings? Does it specify some arbitrary limit?

[PSanetra]

The Java implementation seems also just to use the MAX_ARRAY_SIZE as limit:

avro/lang/java/avro/src/main/java/org/apache/avro/io/BinaryDecoder.java

Lines 302 to 316 in 8f0b8d6

	public Utf8 readString(Utf8 old) throws IOException {
	long length = readLong();
	if (length > MAX_ARRAY_SIZE) {
	throw new UnsupportedOperationException("Cannot read strings longer than " + MAX_ARRAY_SIZE + " bytes");
	}
	if (length < 0L) {
	throw new AvroRuntimeException("Malformed data. Length is negative: " + length);
	}
	Utf8 result = (old != null ? old : new Utf8());
	result.setByteLength((int) length);
	if (0L != length) {
	doReadBytes(result.getBytes(), 0, (int) length);
	}
	return result;
	}

@blachniet can you fix the test for .NET Framework 4.6.1? It is hard for me to test this on windows.

[martin-g]

What is the policy for the supported .NET versions in Avro C# ?
According to https://dotnet.microsoft.com/platform/support/policy/dotnet-core only 3.1 (LTS) and 5.0 (Current) are supported by Microsoft.

[RyanSkraba]

Huh, that's confusing. According to https://docs.microsoft.com/en-us/lifecycle/faq/dotnet-framework#what-is-the-lifecycle-policy-for-different-versions-of--net-framework- there's quite a few supported versions in the 4.x range...

[PSanetra]

@blachniet @RyanSkraba I have changed the constant MaxDotNetArrayLength for .Net Framework 4.6.1 to 0x3FFFFFFF. This seems to work. I could not find any correct official documentation. Thanks @superkartoffel for helping debugging this on windows.

[RyanSkraba]

Hello! These two JIRA seem pretty important to include in the 1.11.0 RC2 -- I tried giving the C# SDK a compile in an Azure Windows VM, but it's a pretty steep learning curve. Can anyone help out?

[RyanSkraba]

This LGTM pragmatically -- since it builds and runs correctly both in Windows and linux when combined with PR#1375
@blachniet are you happy with the changes you've requested? If we can merge these two, I can cherry-pick them to branch-1.11 and create a new release candidate!