Skip to content

Add a Stream buffer validation helper #1605

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

Merged
merged 2 commits into from
Mar 25, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 4 additions & 11 deletions src/Renci.SshNet/Common/ChannelInputStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,10 @@ public override int Read(byte[] buffer, int offset, int count)
/// <exception cref="ArgumentOutOfRangeException">offset or count is negative.</exception>
public override void Write(byte[] buffer, int offset, int count)
{
ThrowHelper.ThrowIfNull(buffer);

if (offset + count > buffer.Length)
{
throw new ArgumentException("The sum of offset and count is greater than the buffer length.");
}

if (offset < 0 || count < 0)
{
throw new ArgumentOutOfRangeException(nameof(offset), "offset or count is negative.");
}
#if !NET
ThrowHelper.
#endif
ValidateBufferArguments(buffer, offset, count);

ThrowHelper.ThrowObjectDisposedIf(_isDisposed, this);

Expand Down
6 changes: 1 addition & 5 deletions src/Renci.SshNet/Common/PacketDump.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ public static string Create(List<byte> data, int indentLevel)
public static string Create(byte[] data, int indentLevel)
{
ThrowHelper.ThrowIfNull(data);

if (indentLevel < 0)
{
throw new ArgumentOutOfRangeException(nameof(indentLevel), "Cannot be less than zero.");
}
ThrowHelper.ThrowIfNegative(indentLevel);

const int lineWidth = 16;

Expand Down
10 changes: 10 additions & 0 deletions src/Renci.SshNet/Common/PipeStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,11 @@ public override void SetLength(long value)
/// <inheritdoc/>
public override int Read(byte[] buffer, int offset, int count)
{
#if !NET
ThrowHelper.
#endif
ValidateBufferArguments(buffer, offset, count);

lock (_sync)
{
while (_head == _tail && !_disposed)
Expand All @@ -88,6 +93,11 @@ public override int Read(byte[] buffer, int offset, int count)
/// <inheritdoc/>
public override void Write(byte[] buffer, int offset, int count)
{
#if !NET
ThrowHelper.
#endif
ValidateBufferArguments(buffer, offset, count);

lock (_sync)
{
ThrowHelper.ThrowObjectDisposedIf(_disposed, this);
Expand Down
42 changes: 42 additions & 0 deletions src/Renci.SshNet/Common/ThrowHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,48 @@ static void Throw(string? argument, string? paramName)
throw new ArgumentException("The value cannot be an empty string.", paramName);
}
}
#endif
}

#if !NET
// A rough copy of
// https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs#L960C13-L974C10
// for lower targets.
public static void ValidateBufferArguments(byte[] buffer, int offset, int count)
{
ThrowIfNull(buffer);
ThrowIfNegative(offset);

if ((uint)count > buffer.Length - offset)
{
Throw();

[DoesNotReturn]
static void Throw()
{
throw new ArgumentOutOfRangeException(
nameof(count),
"Offset and length were out of bounds for the array or count is greater than the number of elements from index to the end of the source collection.");
}
}
}
#endif

public static void ThrowIfNegative(long value, [CallerArgumentExpression(nameof(value))] string? paramName = null)
{
#if NET
ArgumentOutOfRangeException.ThrowIfNegative(value, paramName);
#else
if (value < 0)
{
Throw(value, paramName);

[DoesNotReturn]
static void Throw(long value, string? paramName)
{
throw new ArgumentOutOfRangeException(paramName, value, "Value must be non-negative.");
}
}
#endif
}
}
Expand Down
107 changes: 20 additions & 87 deletions src/Renci.SshNet/Sftp/SftpFileStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -511,28 +511,12 @@ public override Task FlushAsync(CancellationToken cancellationToken)
/// </remarks>
public override int Read(byte[] buffer, int offset, int count)
{
var readLen = 0;

ThrowHelper.ThrowIfNull(buffer);

#if NET
ArgumentOutOfRangeException.ThrowIfNegative(offset);
ArgumentOutOfRangeException.ThrowIfNegative(count);
#else
if (offset < 0)
{
throw new ArgumentOutOfRangeException(nameof(offset));
}

if (count < 0)
{
throw new ArgumentOutOfRangeException(nameof(count));
}
#if !NET
ThrowHelper.
#endif
if ((buffer.Length - offset) < count)
{
throw new ArgumentException("Invalid array range.");
}
ValidateBufferArguments(buffer, offset, count);

var readLen = 0;

// Lock down the file stream while we do this.
lock (_lock)
Expand Down Expand Up @@ -653,28 +637,14 @@ public override int Read(byte[] buffer, int offset, int count)
/// <returns>A <see cref="Task" /> that represents the asynchronous read operation.</returns>
public override async Task<int> ReadAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
var readLen = 0;

ThrowHelper.ThrowIfNull(buffer);
#if !NET
ThrowHelper.
#endif
ValidateBufferArguments(buffer, offset, count);

#if NET
ArgumentOutOfRangeException.ThrowIfNegative(offset);
ArgumentOutOfRangeException.ThrowIfNegative(count);
#else
if (offset < 0)
{
throw new ArgumentOutOfRangeException(nameof(offset));
}
cancellationToken.ThrowIfCancellationRequested();

if (count < 0)
{
throw new ArgumentOutOfRangeException(nameof(count));
}
#endif
if ((buffer.Length - offset) < count)
{
throw new ArgumentException("Invalid array range.");
}
var readLen = 0;

CheckSessionIsOpen();

Expand Down Expand Up @@ -952,14 +922,7 @@ public override long Seek(long offset, SeekOrigin origin)
/// </remarks>
public override void SetLength(long value)
{
#if NET
ArgumentOutOfRangeException.ThrowIfNegative(value);
#else
if (value < 0)
{
throw new ArgumentOutOfRangeException(nameof(value));
}
#endif
ThrowHelper.ThrowIfNegative(value);

// Lock down the file stream while we do this.
lock (_lock)
Expand Down Expand Up @@ -1005,26 +968,10 @@ public override void SetLength(long value)
/// <exception cref="ObjectDisposedException">Methods were called after the stream was closed.</exception>
public override void Write(byte[] buffer, int offset, int count)
{
ThrowHelper.ThrowIfNull(buffer);

#if NET
ArgumentOutOfRangeException.ThrowIfNegative(offset);
ArgumentOutOfRangeException.ThrowIfNegative(count);
#else
if (offset < 0)
{
throw new ArgumentOutOfRangeException(nameof(offset));
}

if (count < 0)
{
throw new ArgumentOutOfRangeException(nameof(count));
}
#if !NET
ThrowHelper.
#endif
if ((buffer.Length - offset) < count)
{
throw new ArgumentException("Invalid array range.");
}
ValidateBufferArguments(buffer, offset, count);

// Lock down the file stream while we do this.
lock (_lock)
Expand Down Expand Up @@ -1105,26 +1052,12 @@ public override void Write(byte[] buffer, int offset, int count)
/// <exception cref="ObjectDisposedException">Methods were called after the stream was closed.</exception>
public override async Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken)
{
ThrowHelper.ThrowIfNull(buffer);

#if NET
ArgumentOutOfRangeException.ThrowIfNegative(offset);
ArgumentOutOfRangeException.ThrowIfNegative(count);
#else
if (offset < 0)
{
throw new ArgumentOutOfRangeException(nameof(offset));
}

if (count < 0)
{
throw new ArgumentOutOfRangeException(nameof(count));
}
#if !NET
ThrowHelper.
#endif
if ((buffer.Length - offset) < count)
{
throw new ArgumentException("Invalid array range.");
}
ValidateBufferArguments(buffer, offset, count);

cancellationToken.ThrowIfCancellationRequested();

CheckSessionIsOpen();

Expand Down
10 changes: 10 additions & 0 deletions src/Renci.SshNet/ShellStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -789,6 +789,11 @@ public string Read()
/// <inheritdoc/>
public override int Read(byte[] buffer, int offset, int count)
{
#if !NET
ThrowHelper.
#endif
ValidateBufferArguments(buffer, offset, count);

lock (_sync)
{
while (_readHead == _readTail && !_disposed)
Expand Down Expand Up @@ -835,6 +840,11 @@ public void Write(string? text)
/// <inheritdoc/>
public override void Write(byte[] buffer, int offset, int count)
{
#if !NET
ThrowHelper.
#endif
ValidateBufferArguments(buffer, offset, count);

ThrowHelper.ThrowObjectDisposedIf(_disposed, this);

while (count > 0)
Expand Down
3 changes: 0 additions & 3 deletions test/Renci.SshNet.Tests/Classes/Common/PacketDumpTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using Microsoft.VisualStudio.TestTools.UnitTesting;

using Renci.SshNet.Common;
using Renci.SshNet.Tests.Common;

namespace Renci.SshNet.Tests.Classes.Common
{
Expand Down Expand Up @@ -41,8 +40,6 @@ public void Create_ByteArrayAndIndentLevel_IndentLevelLessThanZero()
{
Assert.IsNull(ex.InnerException);

ArgumentExceptionAssert.MessageEquals("Cannot be less than zero.", ex);

Assert.AreEqual("indentLevel", ex.ParamName);
}
}
Expand Down