Skip to content

Commit

Permalink
Fix avoidable exception when data length is too long (#823)
Browse files Browse the repository at this point in the history
* Data lengths longer than stream position when data lengths are greater than int.maxvalue are ignored and do not throw an exception

* Removed unreachable test

* Do not try to load the data (just ignore it)

---------

Co-authored-by: Steve Evans <[email protected]>
Co-authored-by: Rob Hague <[email protected]>
  • Loading branch information
3 people authored Nov 19, 2023
1 parent f9f2b0e commit daa1acc
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 61 deletions.
21 changes: 3 additions & 18 deletions src/Renci.SshNet/Messages/Transport/IgnoreMessage.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
using System;
using System.Globalization;
using Renci.SshNet.Abstractions;

namespace Renci.SshNet.Messages.Transport
{
Expand All @@ -13,7 +11,8 @@ public class IgnoreMessage : Message
internal const byte MessageNumber = 2;

/// <summary>
/// Gets ignore message data if any.
/// Gets ignore message data if this message has been initialised
/// with data to be sent. Otherwise, returns an empty array.
/// </summary>
public byte[] Data { get; private set; }

Expand Down Expand Up @@ -61,21 +60,7 @@ protected override int BufferCapacity
/// </summary>
protected override void LoadData()
{
var dataLength = ReadUInt32();
if (dataLength > int.MaxValue)
{
throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, "Data longer than {0} is not supported.", int.MaxValue));
}

if (dataLength > (DataStream.Length - DataStream.Position))
{
DiagnosticAbstraction.Log("SSH_MSG_IGNORE: Length exceeds data bytes, data ignored.");
Data = Array.Empty<byte>();
}
else
{
Data = ReadBytes((int) dataLength);
}
// Do nothing - this data is supposed to be ignored.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System;
using System.Globalization;
using System.Linq;
using Renci.SshNet.Common;
using Renci.SshNet.Messages.Transport;
Expand Down Expand Up @@ -80,7 +79,7 @@ public void GetBytes()
}

[TestMethod]
public void Load()
public void Load_IgnoresData()
{
var ignoreMessage = new IgnoreMessage(_data);
var bytes = ignoreMessage.GetBytes();
Expand All @@ -89,47 +88,7 @@ public void Load()
target.Load(bytes, 1, bytes.Length - 1);

Assert.IsNotNull(target.Data);
Assert.AreEqual(_data.Length, target.Data.Length);
Assert.IsTrue(target.Data.SequenceEqual(_data));
}

[TestMethod]
public void Load_ShouldIgnoreDataWhenItsLengthIsGreatherThanItsActualBytes()
{
var ssh = new SshDataStream(1);
ssh.WriteByte(2); // Type
ssh.Write(5u); // Data length
ssh.Write(new byte[3]); // Data

var ignoreMessageBytes = ssh.ToArray();

var ignoreMessage = new IgnoreMessage();
ignoreMessage.Load(ignoreMessageBytes, 1, ignoreMessageBytes.Length - 1);
Assert.IsNotNull(ignoreMessage.Data);
Assert.AreEqual(0, ignoreMessage.Data.Length);
}

[TestMethod]
public void Load_ShouldThrowNotSupportedExceptionWhenDataLengthIsGreaterThanInt32MaxValue()
{
var ssh = new SshDataStream(1);
ssh.WriteByte(2); // Type
ssh.Write(uint.MaxValue); // Data length
ssh.Write(new byte[3]);

var ignoreMessageBytes = ssh.ToArray();
var ignoreMessage = new IgnoreMessage();

try
{
ignoreMessage.Load(ignoreMessageBytes, 1, ignoreMessageBytes.Length - 1);
Assert.Fail();
}
catch (NotSupportedException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(string.Format(CultureInfo.CurrentCulture, "Data longer than {0} is not supported.", int.MaxValue), ex.Message);
}
Assert.AreEqual(0, target.Data.Length);
}
}
}

0 comments on commit daa1acc

Please sign in to comment.