From 03023911c80b9eb93bd8fe2019e664ffd50ed672 Mon Sep 17 00:00:00 2001 From: Rob Hague Date: Mon, 12 Oct 2026 22:40:34 +0200 Subject: [PATCH] Validate collection sizes before allocating Validate cases where (in particular) a few bytes of server data determines the size of a collection allocated, and throw if it's not possible based on the size of the buffer that the collection is purported to be contained in. --- src/Renci.SshNet/Common/SshDataStream.cs | 9 ++-- .../Sftp/Responses/SftpNameResponse.cs | 10 ++++ src/Renci.SshNet/Sftp/SftpFileAttributes.cs | 12 ++++- .../Classes/Common/SshDataTest.cs | 16 ++----- .../Sftp/Responses/SftpNameResponseTest.cs | 47 +++++++++++++++++++ .../Classes/Sftp/SftpFileAttributesTest.cs | 13 +++++ 6 files changed, 87 insertions(+), 20 deletions(-) create mode 100644 test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpNameResponseTest.cs diff --git a/src/Renci.SshNet/Common/SshDataStream.cs b/src/Renci.SshNet/Common/SshDataStream.cs index 291839c61..ede5a716e 100644 --- a/src/Renci.SshNet/Common/SshDataStream.cs +++ b/src/Renci.SshNet/Common/SshDataStream.cs @@ -371,12 +371,9 @@ public override byte[] ToArray() /// is greater than the internal buffer size. internal byte[] ReadBytes(int length) { - var data = new byte[length]; - var bytesRead = Read(data, 0, length); - if (bytesRead < length) - { - throw new ArgumentOutOfRangeException(nameof(length), string.Format(CultureInfo.InvariantCulture, "The requested length ({0}) is greater than the actual number of bytes read ({1}).", length, bytesRead)); - } + var data = GetRemainingBuffer().Slice(0, length).ToArray(); + + Position += length; return data; } diff --git a/src/Renci.SshNet/Sftp/Responses/SftpNameResponse.cs b/src/Renci.SshNet/Sftp/Responses/SftpNameResponse.cs index 0e6c9f464..a7e6b8e87 100644 --- a/src/Renci.SshNet/Sftp/Responses/SftpNameResponse.cs +++ b/src/Renci.SshNet/Sftp/Responses/SftpNameResponse.cs @@ -1,7 +1,10 @@ using System; using System.Collections.Generic; +using System.Globalization; using System.Text; +using Renci.SshNet.Common; + namespace Renci.SshNet.Sftp.Responses { internal sealed class SftpNameResponse : SftpResponse @@ -29,6 +32,13 @@ protected override void LoadData() base.LoadData(); Count = ReadUInt32(); + + // Each entry is at least two uints in size. + if (Count > (DataStream.Length - DataStream.Position) / 8) + { + throw new SshException(string.Format(CultureInfo.InvariantCulture, "Invalid SSH_FXP_NAME count: {0}.", Count)); + } + Files = new KeyValuePair[Count]; for (var i = 0; i < Count; i++) diff --git a/src/Renci.SshNet/Sftp/SftpFileAttributes.cs b/src/Renci.SshNet/Sftp/SftpFileAttributes.cs index 70ca0ddca..3b14aaedf 100644 --- a/src/Renci.SshNet/Sftp/SftpFileAttributes.cs +++ b/src/Renci.SshNet/Sftp/SftpFileAttributes.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Globalization; using System.Linq; using System.Text; @@ -820,8 +821,15 @@ internal static SftpFileAttributes FromBytes(SshDataStream stream) if ((flag & SSH_FILEXFER_ATTR_EXTENDED) == SSH_FILEXFER_ATTR_EXTENDED) { - var extendedCount = (int)stream.ReadUInt32(); - extensions = new Dictionary(extendedCount); + var extendedCount = stream.ReadUInt32(); + + // Each extension is at least two string lengths. + if (extendedCount > (stream.Length - stream.Position) / 8) + { + throw new SshException(string.Format(CultureInfo.InvariantCulture, "Invalid extended attribute count: {0}.", extendedCount)); + } + + extensions = new Dictionary((int)extendedCount); for (var i = 0; i < extendedCount; i++) { var extensionName = stream.ReadString(Encoding.UTF8); diff --git a/test/Renci.SshNet.Tests/Classes/Common/SshDataTest.cs b/test/Renci.SshNet.Tests/Classes/Common/SshDataTest.cs index 1518dda11..376f73a94 100644 --- a/test/Renci.SshNet.Tests/Classes/Common/SshDataTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Common/SshDataTest.cs @@ -102,23 +102,15 @@ public void ReadBytes_Length_LengthEqualsNumberOfBytesAvailable() } [TestMethod] - public void ReadBytes_Length_LengthIsGreaterThanNumberOfBytesAvailable() + [DataRow(4)] + [DataRow(int.MaxValue)] + public void ReadBytes_Length_LengthIsGreaterThanNumberOfBytesAvailable(int length) { MySshData sshData = new MySshData(); sshData.Load(new byte[] { 0x01, 0x0d, 0x0a }, 0, 3); - try - { - sshData.ReadBytes(4); - Assert.Fail(); - } - catch (ArgumentOutOfRangeException ex) - { - Assert.AreEqual(typeof(ArgumentOutOfRangeException), ex.GetType()); - Assert.IsNull(ex.InnerException); - Assert.AreEqual("length", ex.ParamName); - } + Assert.Throws(() => sshData.ReadBytes(length)); } [TestMethod] diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpNameResponseTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpNameResponseTest.cs new file mode 100644 index 000000000..568d3fb74 --- /dev/null +++ b/test/Renci.SshNet.Tests/Classes/Sftp/Responses/SftpNameResponseTest.cs @@ -0,0 +1,47 @@ +using System.Text; + +using Microsoft.VisualStudio.TestTools.UnitTesting; + +using Renci.SshNet.Common; +using Renci.SshNet.Sftp.Responses; + +namespace Renci.SshNet.Tests.Classes.Sftp.Responses +{ + [TestClass] + public class SftpNameResponseTest + { + [TestMethod] + public void Load() + { + var target = new SftpNameResponse(3, Encoding.UTF8); + + target.Load([ + 0, 0, 0, 7, // response id + 0, 0, 0, 1, // count + 0, 0, 0, 1, (byte)'a', // filename + 0, 0, 0, 0, // longname + 0, 0, 0, 0, // attribute flags + ]); + + Assert.AreEqual(7u, target.ResponseId); + Assert.HasCount(1, target.Files); + Assert.AreEqual("a", target.Files[0].Key); + } + + [TestMethod] + public void Load_CountExceedsData_Throws() + { + var target = new SftpNameResponse(3, Encoding.UTF8); + + var ex = Assert.ThrowsExactly(() => target.Load([ + 0, 0, 0, 7, // response id + 0xFF, 0xFF, 0xFF, 0xFF, // count + 0, 0, 0, 1, (byte)'a', // filename + 0, 0, 0, 0, // longname + 0, 0, 0, 0, // attribute flags + ])); + + Assert.AreEqual("Invalid SSH_FXP_NAME count: 4294967295.", ex.Message); + } + } +} diff --git a/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs b/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs index 566379a74..fa7a1e3f0 100644 --- a/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs +++ b/test/Renci.SshNet.Tests/Classes/Sftp/SftpFileAttributesTest.cs @@ -44,6 +44,19 @@ public void FileTypePropertiesAreMutuallyExclusive( Assert.AreEqual(isNamedPipe, attributes.IsNamedPipe); } + [TestMethod] + public void FromBytes_ExtendedCountExceedsData_Throws() + { + var ex = Assert.ThrowsExactly(() => SftpFileAttributes.FromBytes([ + 0x80, 0, 0, 0, // flags: extended + 0x7F, 0xFF, 0xFF, 0xFF, // extended count + 0, 0, 0, 1, (byte)'a', // extension name + 0, 0, 0, 1, (byte)'b', // extension data + ])); + + Assert.AreEqual("Invalid extended attribute count: 2147483647.", ex.Message); + } + [TestMethod] public void FromBytesGetBytes() {