System.IO.MemoryStream: ReadByte() and WriteByte() can throw IndexOutOfRangeException

[elgonzo]

(Edit: I updated the issue report, as not only ReadByte() is affected but WriteByte() as well).

Description

When creating a non-resizable MemoryStream from an array region whose start/origin index is larger than zero, the MemoryStream.Position value can be set in a way that leads to IndexOutOfRangeException when calling MemoryStream.ReadByte() or MemoryStream.WriteByte() .

This is due to the assignment of the private _position field in the MemoryStream.Position set accessor:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs

Line 315 in 57608c3

_position = _origin + (int)value;

which can lead to _position becoming negative.

Both the ReadByte() and WriteByte() method, however, do not test for _position being negative before using it as an index into the _buffer byte array, thus being vulnerable to causing IndexOutOfRangeException.

ReadByte():

runtime/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs

Lines 430 to 433 in 57608c3

	if (_position >= _length)
	return -1;
	return _buffer[_position++];

WriteByte():

runtime/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs

Lines 736 to 754 in 57608c3

	if (_position >= _length)
	{
	int newLength = _position + 1;
	bool mustZero = _position > _length;
	if (newLength >= _capacity)
	{
	bool allocatedNewArray = EnsureCapacity(newLength);
	if (allocatedNewArray)
	{
	mustZero = false;
	}
	}
	if (mustZero)
	{
	Array.Clear(_buffer, _length, _position - _length);
	}
	_length = newLength;
	}
	_buffer[_position++] = value;

Reproduction Steps

byte[] buffer = new byte[100];
using var ms = new System.IO.MemoryStream(buffer, 10, buffer.Length - 10, true);

//
// Let the private MemoryStream._position field overflow
//
var newPosition = int.MaxValue - 9;
ms.Position = newPosition;
System.Console.WriteLine("ms.Position == newPosition: " + (ms.Position == newPosition));

System.Console.WriteLine();
System.Console.WriteLine("ReadByte");
try
{
	var b = ms.ReadByte();
	System.Console.WriteLine("ms.ReadByte() returned " + b);
}
catch (System.Exception ex)
{
	System.Console.WriteLine(ex);
}

System.Console.WriteLine();
System.Console.WriteLine("WriteByte");
try
{
	ms.WriteByte(0);
}
catch (System.Exception ex)
{
	System.Console.WriteLine(ex);
}

Expected behavior

Expected result/output:

ms.Position == newPosition: True

ReadByte
ms.ReadByte() returned -1

WriteByte
System.NotSupportedException: Memory stream is not expandable.
 ...

Actual behavior

Actual result/output:

ms.Position == newPosition: True

ReadByte
System.IndexOutOfRangeException: Index was outside the bounds of the array.
 at System.IO.MemoryStream.ReadByte()
 ...

WriteByte
System.IndexOutOfRangeException: Index was outside the bounds of the array.
 at System.IO.MemoryStream.WriteByte(Byte value)
 ...

Regression?

No response

Known Workarounds

No response

Configuration

.NET 8 Preview 3 (https://dotnetfiddle.net/V1POV0)
.NET Framework 4.7.2 (https://dotnetfiddle.net/gM2LkH)

Considering that the bug is already present and reproducible in the old .NET Framework 4.7.2, it's reasonable to assume that at least any .NET version following Framework 4.7.2 is also affected by this issue.

Other information

No response

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

When creating a non-resizable MemoryStream from an array region whose start offset is larger than zero, the MemoryStream.Position value can be set in a way that leads to IndexOutOfRangeException when trying to read the stream.

This is due to the assignment of the private __position field in the MemoryStream.Position set accessor:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs

Line 315 in 57608c3

_position = _origin + (int)value;

which is vulnerable to overflows and thus can lead to _position becoming negative.

Reproduction Steps

static void OutputPrivatePositionField(System.IO.MemoryStream stream)
{
	var fi = typeof(System.IO.MemoryStream).GetField("_position", System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic);
	System.Console.WriteLine($"MemoryStream._position = {fi.GetValue(stream)}");
}


byte[] buffer = new byte[200];
for (int i=0; i < buffer.Length; ++i)
 buffer[i] = (byte) i;
		

using var ms = new System.IOMemoryStream(buffer, 10, buffer.Length - 10, true);
//
// Let the private MemoryStream._position field overflow
//
ms.Position = int.MaxValue - 9;
OutputPrivatePositionField(ms);
var b = ms.ReadByte();
System.Console.WriteLine($"ms.ReadByte() returned {b}");

(Fiddle: https://dotnetfiddle.net/pnTEDr)

Expected behavior

Expected result/output:

MemoryStream._position = <<whatever value>>
ms.ReadByte() returned -1

Actual behavior

Actual result/output:

MemoryStream._position = -2147483648
Unhandled exception. System.IndexOutOfRangeException: Index was outside the bounds of the array.
 at System.IO.MemoryStream.ReadByte()

Regression?

No response

Known Workarounds

No response

Configuration

.NET 8 Preview 3 (https://dotnetfiddle.net/pnTEDr)
.NET Framework 4.7.2 (https://dotnetfiddle.net/FnU7BK)

Considering that the bug is already present and reproducible in the old .NET Framework 4.7.2, it's reasonable to assume that at least any .NET version following Framework 4.7.2 is also affected by this issue.

Other information

No response

Author:	elgonzo
Assignees:	-
Labels:	area-System.IO
Milestone:	-

[manandre]

Thanks for reporting this issue.
The issue here starts you try to set the Position property with an invalid value, an error already exists for this situation:

ArgumentOutOfRangeException: Stream length must be non-negative and less than 2^31 - 1 - origin.

used in

runtime/src/libraries/System.Private.CoreLib/src/System/IO/MemoryStream.cs

Lines 313 to 314 in 3110693

	if (value > MemStreamMaxLength)
	throw new ArgumentOutOfRangeException(nameof(value), SR.ArgumentOutOfRange_StreamLength);

But this code does not take into account the _origin field!
This must be fixed.
It seems that the Seek method is also impacted with the same type of issue.

[elgonzo]

The issue here starts you try to set the Position property with an invalid value, an error already exists for this situation:

ArgumentOutOfRangeException: Stream length must be non-negative and less than 2^31 - 1 - origin.

Your argument is in conflict with the documentation for MemoryStream.Position, which states (https://learn.microsoft.com/en-us/dotnet/api/system.io.memorystream.position?view=net-8.0#exceptions):

Exceptions
ArgumentOutOfRangeException

The position is set to a negative value or a value greater than Int32.MaxValue.

I am in no position to tell whether your argument is mistaken or the documentation is incorrect. But if the team decides that the MemoryStream.Position property should throw for positive values less than or equal to int.MaxValue, then the MemoryStream.Position documentation needs to be updated to correctly reflect the changed behavior.

(To me personally, it would make more sense to keep the MemoryStream.Position behavior as is and adjust the _position checks in the ReadByte() and WriteByte() methods, as this will keep MemoryStream behavior in line with its existing documentation and doesn't require adding more special cases/conditions to the documentation as to when this or that exception is being thrown.)

[manandre]

I agree that the situation is quite confusing here.

• The documentation states that the ArgumentOutOfRangeException must only be raised when the position value is greater than int.MaxValue
• In such a situation, the MemoryStream.Position property currently raises the ArgumentOutOfRangeException with the following message:

Stream length must be non-negative and less than 2^31 - 1 - origin.

The limit of MemoryStream.Position to Int32.MaxValue, while it returns an Int64 (long) type, is only related to the underlying array limitation, what then must take into account the origin : index = _origin + Position < Int32.MaxValue.
From my point of view, the documentation of the ArgumentOutOfRangeException for Position (and Seek) methods should be aligned on the SetLength one (or just removed to strictly follow the Stream contract).

@stephentoub Can you please help here to define the expected behavior in this overflow case?