Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Calling BufferedStream.Write(array, offset, count) where count > 1073741823 results in an OverflowException #50839

Closed
LordBenjamin opened this issue Apr 7, 2021 · 2 comments · Fixed by #53338
Assignees
Labels
area-System.IO help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@LordBenjamin
Copy link

LordBenjamin commented Apr 7, 2021

Description

Calling BufferedStream.Write(array, offset, count) where count > 1073741823 results in an OverflowException.

This works as expected:

byte[] bytes = new byte[int.MaxValue / 2];
using (BufferedStream stream = new BufferedStream(Stream.Null))
{
    stream.Write(bytes, 0, bytes.Length);
}

This throws an OverflowException (note the + 1):

byte[] bytes = new byte[(int.MaxValue / 2) + 1];
using (BufferedStream stream = new BufferedStream(Stream.Null))
{
    stream.Write(bytes, 0, bytes.Length);
}

Expected behaviour is that the above should write successfully and not throw an OverflowException.

Configuration

  • Windows 10
  • VS 2019
  • .NET 5.0

Regression?

Also fails in .NET Framework 4.7.2 and 4.8. Looking at Reference Source, the code seems to be the same.

Other information

I believe this is related to the heuristic check for useBuffer, where count is added twice:

checked
{  // We do not expect buffer sizes big enough for an overflow, but if it happens, lets fail early:
    totalUserbytes = _writePos + count;
    useBuffer = (totalUserbytes + count < (_bufferSize + _bufferSize));
}

If count is larger than Int32.MaxValue / 2, adding it to itself will overflow.

While the comment "we do not expect buffer sizes big enough for an overflow" seems reasonable for the internal buffer, the buffer argument supplied to Write() comes from user code and it doesn't seem intuitive for BufferedStream to accept a different maximum buffer size to other Stream implementations.

Could this check use long arithmetic to avoid the overflow where count * 2 > Int32.MaxValue?

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.IO untriaged New issue has not been triaged by the area owner labels Apr 7, 2021
@ghost
Copy link

ghost commented Apr 7, 2021

Tagging subscribers to this area: @carlossanlop
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Calling BufferedStream.Write(array, offset, count) where count > 1073741823 results in an OverflowException.

This works as expected:

byte[] bytes = new byte[int.MaxValue / 2];
using (BufferedStream stream = new BufferedStream(Stream.Null))
{
    stream.Write(bytes, 0, bytes.Length);
}

This throws an OverflowException (note the + 1):

byte[] bytes = new byte[(int.MaxValue / 2) + 1];
using (BufferedStream stream = new BufferedStream(Stream.Null))
{
    stream.Write(bytes, 0, bytes.Length);
}

Expected behaviour is that the above should not throw an OverflowException.

Configuration

  • Windows 10
  • VS 2019
  • .NET 5.0

Regression?

Also fails in .NET Framework 4.7.2 and 4.8. Looking at Reference Source, the code seems to be the same.

Other information

I believe this is related to the heuristic check for useBuffer, where count is added twice:

checked
{  // We do not expect buffer sizes big enough for an overflow, but if it happens, lets fail early:
    totalUserbytes = _writePos + count;
    useBuffer = (totalUserbytes + count < (_bufferSize + _bufferSize));
}

If count is larger than Int32.MaxValue / 2, adding it to itself will overflow.

While the comment "we do not expect buffer sizes big enough for an overflow" seems reasonable for the internal buffer, the buffer argument supplied to Write() comes from user code and it doesn't seem intuitive for BufferedStream to accept a different maximum buffer size to other Stream implementations.

Could this check use long arithmetic to avoid the overflow where count * 2 > Int32.MaxValue?

Author: LordBenjamin
Assignees: -
Labels:

area-System.IO, untriaged

Milestone: -

@jozkee jozkee added help wanted [up-for-grabs] Good issue for external contributors and removed untriaged New issue has not been triaged by the area owner labels May 20, 2021
@jozkee jozkee added this to the Future milestone May 20, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 27, 2021
@lateapexearlyspeed
Copy link
Contributor

Just raise a small PR #53338 for that. Used uint arithmetic to handle that because it is not possible that sum of two int value be larger than uint.MaxValue.
Any one help review that, thanks!

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 30, 2021
@adamsitnik adamsitnik modified the milestones: Future, 6.0.0 Jul 30, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants