Skip to content

Commit

Permalink
Add missing length check in parsing ACC messages, add more related tests
Browse files Browse the repository at this point in the history
Signed-off-by: Arne Schwabe <[email protected]>
  • Loading branch information
schwabe committed Jan 4, 2024
1 parent 8bfdc28 commit 8ad83b5
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 1 deletion.
10 changes: 9 additions & 1 deletion openvpn/ssl/customcontrolchannel.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
#include <openvpn/common/unicode.hpp>
#include <openvpn/common/base64.hpp>
#include <openvpn/buffer/bufstr.hpp>
#include <openvpn/common/number.hpp>

namespace openvpn {

Expand Down Expand Up @@ -177,15 +178,22 @@ class AppControlMessageReceiver
throw parse_acc_message{"Discarding malformed custom app control message"};
}


auto protocol = std::move(parts[1]);
auto length = std::move(parts[2]);
auto length_str = std::move(parts[2]);
auto flags = std::move(parts[3]);
auto message = std::move(parts[4]);

bool base64Encoding = false;
bool textEncoding = false;
bool fragment = false;

size_t length = 0;
if (!parse_number(length_str, length) || length != message.length())
{
throw parse_acc_message{"Discarding malformed custom app control message"};
}

for (char const &c : flags)
{
switch (c)
Expand Down
44 changes: 44 additions & 0 deletions test/unittests/test_acc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,48 @@ TEST(customcontrolchannel, send_with_nul)

EXPECT_EQ(cmsgs.size(), 1);
EXPECT_EQ(cmsgs[0], expected_control_msg);
}

TEST(customcontrolchannel, test_incorrect_len)
{
std::string control_msg{"ACC,fortune,62,6,InsgIm1lIjogImZyb2ciLCAAeGZm/SJtc2ciOiAiSSBhbSAAS2VybWl0IiB9Ig=="};

AppControlMessageReceiver accrecv{};

EXPECT_THROW(
accrecv.receive_message(control_msg),
parse_acc_message);
}

TEST(customcontrolchannel, test_wrong_header)
{
std::string control_msg{"ABC,fortune,64,6,InsgIm1lIjogImZyb2ciLCAAeGZm/SJtc2ciOiAiSSBhbSAAS2VybWl0IiB9Ig=="};

AppControlMessageReceiver accrecv{};

EXPECT_THROW(
accrecv.receive_message(control_msg),
parse_acc_message);
}

TEST(customcontrolchannel, test_unsupported_encoding)
{
std::string control_msg{"ACC,fortune,64,Q,InsgIm1lIjogImZyb2ciLCAAeGZm/SJtc2ciOiAiSSBhbSAAS2VybWl0IiB9Ig=="};

AppControlMessageReceiver accrecv{};

EXPECT_THROW(
accrecv.receive_message(control_msg),
parse_acc_message);
}

TEST(customcontrolchannel, test_missing_message)
{
std::string control_msg{"ABC,fortune,64,6"};

AppControlMessageReceiver accrecv{};

EXPECT_THROW(
accrecv.receive_message(control_msg),
parse_acc_message);
}

0 comments on commit 8ad83b5

Please sign in to comment.