diff --git a/.github/workflows/ReleaseNotes.md b/.github/workflows/ReleaseNotes.md index e69de29b..1c12c6b7 100644 --- a/.github/workflows/ReleaseNotes.md +++ b/.github/workflows/ReleaseNotes.md @@ -0,0 +1 @@ +* [Core] Add validation of maximum string lengths (#1718). \ No newline at end of file diff --git a/Source/MQTTnet.Benchmarks/MqttPacketReaderWriterBenchmark.cs b/Source/MQTTnet.Benchmarks/MqttPacketReaderWriterBenchmark.cs index a0c474a1..0efc7ffa 100644 --- a/Source/MQTTnet.Benchmarks/MqttPacketReaderWriterBenchmark.cs +++ b/Source/MQTTnet.Benchmarks/MqttPacketReaderWriterBenchmark.cs @@ -30,17 +30,17 @@ namespace MQTTnet.Benchmarks var writer = new MqttBufferWriter(4096, 65535); writer.WriteString("A relative short string."); - writer.WriteBinaryData(_demoPayload); + writer.WriteBinary(_demoPayload); writer.WriteByte(0x01); writer.WriteByte(0x02); writer.WriteVariableByteInteger(5647382); writer.WriteString("A relative short string."); writer.WriteVariableByteInteger(8574489); - writer.WriteBinaryData(_demoPayload); + writer.WriteBinary(_demoPayload); writer.WriteByte(2); writer.WriteByte(0x02); writer.WriteString("fjgffiogfhgfhoihgoireghreghreguhreguireoghreouighreouighreughreguiorehreuiohruiorehreuioghreug"); - writer.WriteBinaryData(_demoPayload); + writer.WriteBinary(_demoPayload); _readPayload = new ArraySegment(writer.GetBuffer(), 0, writer.Length).ToArray(); } @@ -83,11 +83,11 @@ namespace MQTTnet.Benchmarks writer.WriteVariableByteInteger(5647382); writer.WriteString("A relative short string."); writer.WriteVariableByteInteger(8574589); - writer.WriteBinaryData(_demoPayload); + writer.WriteBinary(_demoPayload); writer.WriteByte(2); writer.WriteByte(0x02); writer.WriteString("fjgffiogfhgfhoihgoireghreghreguhreguireoghreouighreouighreughreguiorehreuiohruiorehreuioghreug"); - writer.WriteBinaryData(_demoPayload); + writer.WriteBinary(_demoPayload); writer.Reset(0); } diff --git a/Source/MQTTnet.Tests/Formatter/MqttPacketWriter_Tests.cs b/Source/MQTTnet.Tests/Formatter/MqttPacketWriter_Tests.cs index e2748f03..7a16ff30 100644 --- a/Source/MQTTnet.Tests/Formatter/MqttPacketWriter_Tests.cs +++ b/Source/MQTTnet.Tests/Formatter/MqttPacketWriter_Tests.cs @@ -1,6 +1,7 @@ using System; using System.Linq; using Microsoft.VisualStudio.TestTools.UnitTesting; +using MQTTnet.Exceptions; using MQTTnet.Formatter; namespace MQTTnet.Tests.Formatter @@ -47,23 +48,32 @@ namespace MQTTnet.Tests.Formatter Assert.AreEqual(1234U, reader.ReadVariableByteInteger()); Assert.AreEqual(9876U, reader.ReadVariableByteInteger()); } + + [TestMethod] + [ExpectedException(typeof(MqttProtocolViolationException))] + public void Throw_If_String_Too_Long() + { + var writer = new MqttBufferWriter(4096, 65535); + + writer.WriteString(string.Empty.PadLeft(65536)); + } [TestMethod] public void Write_And_Read_Multiple_Times() { var writer = new MqttBufferWriter(4096, 65535); writer.WriteString("A relative short string."); - writer.WriteBinaryData(new byte[1234]); + writer.WriteBinary(new byte[1234]); writer.WriteByte(0x01); writer.WriteByte(0x02); writer.WriteVariableByteInteger(5647382); writer.WriteString("A relative short string."); writer.WriteVariableByteInteger(8574489); - writer.WriteBinaryData(new byte[48]); + writer.WriteBinary(new byte[48]); writer.WriteByte(2); writer.WriteByte(0x02); writer.WriteString("fjgffiogfhgfhoihgoireghreghreguhreguireoghreouighreouighreughreguiorehreuiohruiorehreuioghreug"); - writer.WriteBinaryData(new byte[3]); + writer.WriteBinary(new byte[3]); var readPayload = new ArraySegment(writer.GetBuffer(), 0, writer.Length).ToArray(); diff --git a/Source/MQTTnet.Tests/MqttPacketWriter_Tests.cs b/Source/MQTTnet.Tests/MqttPacketWriter_Tests.cs index 02eeb40a..0a3d3f5e 100644 --- a/Source/MQTTnet.Tests/MqttPacketWriter_Tests.cs +++ b/Source/MQTTnet.Tests/MqttPacketWriter_Tests.cs @@ -24,10 +24,10 @@ namespace MQTTnet.Tests writer.WriteString("1234567890"); Assert.AreEqual(10 + 2, writer.Length); - writer.WriteBinaryData(new byte[300]); + writer.WriteBinary(new byte[300]); Assert.AreEqual(300 + 2 + 12, writer.Length); - writer.WriteBinaryData(new byte[5000]); + writer.WriteBinary(new byte[5000]); Assert.AreEqual(5000 + 2 + 300 + 2 + 12, writer.Length); } } diff --git a/Source/MQTTnet/Formatter/MqttBufferWriter.cs b/Source/MQTTnet/Formatter/MqttBufferWriter.cs index 0849f5b3..14ad707b 100644 --- a/Source/MQTTnet/Formatter/MqttBufferWriter.cs +++ b/Source/MQTTnet/Formatter/MqttBufferWriter.cs @@ -19,7 +19,8 @@ namespace MQTTnet.Formatter /// public sealed class MqttBufferWriter { - public const uint VariableByteIntegerMaxValue = 268435455; + const uint VariableByteIntegerMaxValue = 268435455; + const int EncodedStringMaxLength = 65535; readonly int _maxBufferSize; @@ -112,25 +113,7 @@ namespace MQTTnet.Formatter WriteBinary(propertyWriter._buffer, 0, propertyWriter.Length); } - public void WriteBinary(byte[] buffer, int offset, int count) - { - if (buffer == null) - { - throw new ArgumentNullException(nameof(buffer)); - } - - if (count == 0) - { - return; - } - - EnsureAdditionalCapacity(count); - - MqttMemoryHelper.Copy(buffer, offset, _buffer, _position, count); - IncreasePosition(count); - } - - public void WriteBinaryData(byte[] value) + public void WriteBinary(byte[] value) { if (value == null || value.Length == 0) { @@ -155,6 +138,24 @@ namespace MQTTnet.Formatter } } + public void WriteBinary(byte[] buffer, int offset, int count) + { + if (buffer == null) + { + throw new ArgumentNullException(nameof(buffer)); + } + + if (count == 0) + { + return; + } + + EnsureAdditionalCapacity(count); + + MqttMemoryHelper.Copy(buffer, offset, _buffer, _position, count); + IncreasePosition(count); + } + public void WriteByte(byte @byte) { EnsureAdditionalCapacity(1); @@ -186,6 +187,13 @@ namespace MQTTnet.Formatter var writtenBytes = Encoding.UTF8.GetBytes(value, 0, value.Length, _buffer, _position + 2); + // From RFC: 1.5.4 UTF-8 Encoded String + // Unless stated otherwise all UTF-8 encoded strings can have any length in the range 0 to 65,535 bytes. + if (writtenBytes > EncodedStringMaxLength) + { + throw new MqttProtocolViolationException($"The maximum string length is 65535. The current string has a length of {writtenBytes}."); + } + _buffer[_position] = (byte)(writtenBytes >> 8); _buffer[_position + 1] = (byte)writtenBytes; @@ -273,8 +281,7 @@ namespace MQTTnet.Formatter newBufferLength *= 2; } - // Array.Resize will create a new array and copy the existing - // data to the new one. + // Array.Resize will create a new array and copy the existing data to the new one. Array.Resize(ref _buffer, newBufferLength); } @@ -285,6 +292,8 @@ namespace MQTTnet.Formatter if (_position > Length) { + // Also extend the position because we reached the end of the + // pre allocated buffer. Length = _position; } } diff --git a/Source/MQTTnet/Formatter/MqttPacketFormatterAdapter.cs b/Source/MQTTnet/Formatter/MqttPacketFormatterAdapter.cs index 580c59b3..af4e4976 100644 --- a/Source/MQTTnet/Formatter/MqttPacketFormatterAdapter.cs +++ b/Source/MQTTnet/Formatter/MqttPacketFormatterAdapter.cs @@ -31,10 +31,14 @@ namespace MQTTnet.Formatter public MqttProtocolVersion ProtocolVersion { get; private set; } = MqttProtocolVersion.Unknown; + public void Cleanup() + { + _bufferWriter.Cleanup(); + } + public MqttPacket Decode(ReceivedMqttPacket receivedMqttPacket) { ThrowIfFormatterNotSet(); - return _formatter.Decode(receivedMqttPacket); } @@ -50,11 +54,6 @@ namespace MQTTnet.Formatter return _formatter.Encode(packet); } - public void Cleanup() - { - _bufferWriter.Cleanup(); - } - public static IMqttPacketFormatter GetMqttPacketFormatter(MqttProtocolVersion protocolVersion, MqttBufferWriter bufferWriter) { if (protocolVersion == MqttProtocolVersion.Unknown) @@ -98,7 +97,7 @@ namespace MQTTnet.Formatter // Remove the mosquitto try_private flag (MQTT 3.1.1 Bridge). // This flag is accepted but not yet used. protocolLevel &= 0x7F; - + if (protocolName == "MQTT") { if (protocolLevel == 5) diff --git a/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs b/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs index 92fbb836..808140b4 100644 --- a/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs +++ b/Source/MQTTnet/Formatter/V3/MqttV3PacketFormatter.cs @@ -459,7 +459,7 @@ namespace MQTTnet.Formatter.V3 if (packet.WillFlag) { bufferWriter.WriteString(packet.WillTopic); - bufferWriter.WriteBinaryData(packet.WillMessage); + bufferWriter.WriteBinary(packet.WillMessage); } if (packet.Username != null) @@ -469,7 +469,7 @@ namespace MQTTnet.Formatter.V3 if (packet.Password != null) { - bufferWriter.WriteBinaryData(packet.Password); + bufferWriter.WriteBinary(packet.Password); } return MqttBufferWriter.BuildFixedHeader(MqttControlPacketType.Connect); @@ -521,7 +521,7 @@ namespace MQTTnet.Formatter.V3 if (packet.WillFlag) { bufferWriter.WriteString(packet.WillTopic); - bufferWriter.WriteBinaryData(packet.WillMessage); + bufferWriter.WriteBinary(packet.WillMessage); } if (packet.Username != null) @@ -531,7 +531,7 @@ namespace MQTTnet.Formatter.V3 if (packet.Password != null) { - bufferWriter.WriteBinaryData(packet.Password); + bufferWriter.WriteBinary(packet.Password); } return MqttBufferWriter.BuildFixedHeader(MqttControlPacketType.Connect); diff --git a/Source/MQTTnet/Formatter/V5/MqttV5PacketEncoder.cs b/Source/MQTTnet/Formatter/V5/MqttV5PacketEncoder.cs index eaddaaad..8023c9c0 100644 --- a/Source/MQTTnet/Formatter/V5/MqttV5PacketEncoder.cs +++ b/Source/MQTTnet/Formatter/V5/MqttV5PacketEncoder.cs @@ -186,7 +186,7 @@ namespace MQTTnet.Formatter.V5 _propertiesWriter.Reset(); _bufferWriter.WriteString(packet.WillTopic); - _bufferWriter.WriteBinaryData(packet.WillMessage); + _bufferWriter.WriteBinary(packet.WillMessage); } if (packet.Username != null) @@ -196,7 +196,7 @@ namespace MQTTnet.Formatter.V5 if (packet.Password != null) { - _bufferWriter.WriteBinaryData(packet.Password); + _bufferWriter.WriteBinary(packet.Password); } return MqttBufferWriter.BuildFixedHeader(MqttControlPacketType.Connect); diff --git a/Source/MQTTnet/Formatter/V5/MqttV5PropertiesWriter.cs b/Source/MQTTnet/Formatter/V5/MqttV5PropertiesWriter.cs index 30da6de8..29c06566 100644 --- a/Source/MQTTnet/Formatter/V5/MqttV5PropertiesWriter.cs +++ b/Source/MQTTnet/Formatter/V5/MqttV5PropertiesWriter.cs @@ -335,7 +335,7 @@ namespace MQTTnet.Formatter.V5 } _bufferWriter.WriteByte((byte)id); - _bufferWriter.WriteBinaryData(value); + _bufferWriter.WriteBinary(value); } void WriteAsFourByteInteger(MqttPropertyId id, uint value)