Fix avoidable exception when data length is too long (#823)

* 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 <sevans1@oeconnection.com>
Co-authored-by: Rob Hague <rob.hague00@gmail.com>
This commit is contained in:
se006
2023-11-19 01:32:08 -05:00
committed by GitHub
parent f9f2b0e5f4
commit daa1accaf6
2 changed files with 5 additions and 61 deletions
@@ -1,6 +1,4 @@
using System;
using System.Globalization;
using Renci.SshNet.Abstractions;
namespace Renci.SshNet.Messages.Transport
{
@@ -13,7 +11,8 @@ namespace Renci.SshNet.Messages.Transport
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; }
@@ -61,21 +60,7 @@ namespace Renci.SshNet.Messages.Transport
/// </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>
@@ -1,5 +1,4 @@
using System;
using System.Globalization;
using System.Linq;
using Renci.SshNet.Common;
using Renci.SshNet.Messages.Transport;
@@ -80,7 +79,7 @@ namespace Renci.SshNet.Tests.Classes.Messages.Transport
}
[TestMethod]
public void Load()
public void Load_IgnoresData()
{
var ignoreMessage = new IgnoreMessage(_data);
var bytes = ignoreMessage.GetBytes();
@@ -89,47 +88,7 @@ namespace Renci.SshNet.Tests.Classes.Messages.Transport
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);
}
}
}