Skip to content

Commit

Permalink
Fix issue where message is assumed to contain correct length byte (le…
Browse files Browse the repository at this point in the history
…ading to process exit) (#1050)

Do not assume that message contains correct length to prevent calling GetString with invalid parameters.
  • Loading branch information
mnmr authored Mar 16, 2023
1 parent 40631c0 commit 40504cd
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 6 deletions.
55 changes: 55 additions & 0 deletions src/NetMQ.Tests/MechanismTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using System.Text;
using NetMQ.Core.Mechanisms;
using Xunit;

namespace NetMQ.Tests
{
public class MechanismTests
{
private Msg CreateMsg(string data, int lengthDiff)
{
Assert.NotNull(data);
Assert.True(data.Length > 1);
var length = data.Length + lengthDiff;
Assert.True(length > 0);
Assert.True(length < byte.MaxValue - 1);

var msg = new Msg();
msg.InitGC(new byte[data.Length + 1], 0, data.Length + 1);
msg.SetFlags(MsgFlags.Command);
msg.Put((byte)length);
msg.Put(Encoding.ASCII, data, 1);
return msg;
}

[Fact]
public void IsCommandShouldReturnTrueForValidCommand()
{
var mechanism = new NullMechanism(null!, null!);
var msg = CreateMsg("READY", 0);
Assert.True(mechanism.IsCommand("READY", ref msg));
}

[Fact]
public void IsCommandShouldReturnFalseForInvalidCommand()
{
var mechanism = new NullMechanism(null!, null!);
var msg = CreateMsg("READY", -1);
Assert.False(mechanism.IsCommand("READY", ref msg));
msg = CreateMsg("READY", 1);
Assert.False(mechanism.IsCommand("READY", ref msg));
// this test case would fail due to an exception being throw (in 4.0.1.10 and prior)
msg = CreateMsg("READY", 2);
Assert.False(mechanism.IsCommand("READY", ref msg));
}

// this test was used to validate the behavior prior to changing the validation logic in Mechanism.IsCommand
// [Fact]
// public void IsCommandShouldThrowWhenLengthByteExceedsSize()
// {
// var mechanism = new NullMechanism(null, null);
// var msg = CreateMsg("READY", 2);
// Assert.Throws<ArgumentOutOfRangeException>(() => mechanism.IsCommand("READY", ref msg));
// }
}
}
10 changes: 4 additions & 6 deletions src/NetMQ/Core/Mechanisms/Mechanism.cs
Original file line number Diff line number Diff line change
Expand Up @@ -348,15 +348,13 @@ protected bool CheckBasicCommandStructure(ref Msg msg)
return true;
}

protected bool IsCommand(string command, ref Msg msg)
internal protected bool IsCommand(string command, ref Msg msg)
{
if (msg.Size >= command.Length + 1)
if (msg.Size >= command.Length + 1 && msg[0] == command.Length)
{
string msgCommand = msg.GetString(Encoding.ASCII, 1, msg[0]);
return msgCommand == command && msg[0] == command.Length;
return msg.GetString(Encoding.ASCII, 1, msg[0]) == command;
}

return false;
}
}
}
}

0 comments on commit 40504cd

Please sign in to comment.