From daa1accaf677a6382973cb7ac891f31ed7808af6 Mon Sep 17 00:00:00 2001 From: se006 <84139931+se006@users.noreply.github.com> Date: Sun, 19 Nov 2023 01:32:08 -0500 Subject: [PATCH] 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 Co-authored-by: Rob Hague --- .../Messages/Transport/IgnoreMessage.cs | 21 ++------- .../Messages/Transport/IgnoreMessageTest.cs | 45 +------------------ 2 files changed, 5 insertions(+), 61 deletions(-) diff --git a/src/Renci.SshNet/Messages/Transport/IgnoreMessage.cs b/src/Renci.SshNet/Messages/Transport/IgnoreMessage.cs index 5d07f1d0..d657d2d4 100644 --- a/src/Renci.SshNet/Messages/Transport/IgnoreMessage.cs +++ b/src/Renci.SshNet/Messages/Transport/IgnoreMessage.cs @@ -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; /// - /// 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. /// public byte[] Data { get; private set; } @@ -61,21 +60,7 @@ namespace Renci.SshNet.Messages.Transport /// 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(); - } - else - { - Data = ReadBytes((int) dataLength); - } + // Do nothing - this data is supposed to be ignored. } /// diff --git a/test/Renci.SshNet.Tests/Classes/Messages/Transport/IgnoreMessageTest.cs b/test/Renci.SshNet.Tests/Classes/Messages/Transport/IgnoreMessageTest.cs index 1c82d3a6..ecdbf331 100644 --- a/test/Renci.SshNet.Tests/Classes/Messages/Transport/IgnoreMessageTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Messages/Transport/IgnoreMessageTest.cs @@ -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); } } }