SrvRqst: Add bounds checking on buffer parsing
There are multiple length fields provided by the user in this command.
Need to ensure those lengths do not exceed the input buffer size.
Tested:
- Executed new unit tests successfully
Change-Id: Ife4b74d7541d6939bdc30a381fc5cc80cfd24057
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
diff --git a/slp_meta.hpp b/slp_meta.hpp
index aab2f56..5861970 100644
--- a/slp_meta.hpp
+++ b/slp_meta.hpp
@@ -69,7 +69,7 @@
{
constexpr size_t MIN_SRVTYPE_LEN = 22;
-constexpr size_t MIN_SRV_LEN = 26;
+constexpr size_t MIN_SRV_LEN = 24;
constexpr size_t SIZE_PRLIST = 2;
constexpr size_t SIZE_NAMING = 2;
diff --git a/slp_parser.cpp b/slp_parser.cpp
index f99b527..8901726 100644
--- a/slp_parser.cpp
+++ b/slp_parser.cpp
@@ -233,13 +233,27 @@
/* 1) Parse the PRList. */
uint16_t prListLen;
- std::copy_n(buff.data() + slp::request::OFFSET_PR_LEN,
- slp::request::SIZE_PRLIST, (uint8_t*)&prListLen);
+ uint32_t pos = slp::header::MIN_LEN + req.header.langtagLen;
+ if ((pos + slp::request::SIZE_PRLIST) > buff.size())
+ {
+ std::cerr << "PRList length field is greater then input buffer: "
+ << (pos + slp::request::SIZE_PRLIST) << " / " << buff.size()
+ << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
+ std::copy_n(buff.data() + pos, slp::request::SIZE_PRLIST,
+ (uint8_t*)&prListLen);
- auto pos = slp::request::OFFSET_PR_LEN + slp::request::SIZE_PRLIST;
+ pos += slp::request::SIZE_PRLIST;
prListLen = endian::from_network(prListLen);
-
+ if ((pos + prListLen) > buff.size())
+ {
+ std::cerr << "Length of PRList is greater then input buffer: "
+ << (slp::request::OFFSET_PR + prListLen) << " / "
+ << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
req.body.srvrqst.prList.insert(0, (const char*)buff.data() + pos,
prListLen);
@@ -247,6 +261,13 @@
/* 2) Parse the <service-type> string. */
uint16_t srvTypeLen;
+ if ((pos + slp::request::SIZE_SERVICE_TYPE) > buff.size())
+ {
+ std::cerr << "SrvType length field is greater then input buffer: "
+ << (pos + slp::request::SIZE_SERVICE_TYPE) << " / "
+ << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
std::copy_n(buff.data() + pos, slp::request::SIZE_SERVICE_TYPE,
(uint8_t*)&srvTypeLen);
@@ -254,6 +275,12 @@
pos += slp::request::SIZE_SERVICE_TYPE;
+ if ((pos + srvTypeLen) > buff.size())
+ {
+ std::cerr << "Length of SrvType is greater then input buffer: "
+ << (pos + srvTypeLen) << " / " << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
req.body.srvrqst.srvType.insert(0, (const char*)buff.data() + pos,
srvTypeLen);
@@ -261,6 +288,13 @@
/* 3) Parse the <scope-list> string. */
uint16_t scopeListLen;
+ if ((pos + slp::request::SIZE_SCOPE) > buff.size())
+ {
+ std::cerr << "Scope List length field is greater then input buffer: "
+ << (pos + slp::request::SIZE_SCOPE) << " / " << buff.size()
+ << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
std::copy_n(buff.data() + pos, slp::request::SIZE_SCOPE,
(uint8_t*)&scopeListLen);
@@ -268,6 +302,12 @@
pos += slp::request::SIZE_SCOPE;
+ if ((pos + scopeListLen) > buff.size())
+ {
+ std::cerr << "Length of Scope List is greater then input buffer: "
+ << (pos + scopeListLen) << " / " << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
req.body.srvrqst.scopeList.insert(0, (const char*)buff.data() + pos,
scopeListLen);
@@ -275,12 +315,25 @@
/* 4) Parse the <predicate> string. */
uint16_t predicateLen;
+ if ((pos + slp::request::SIZE_PREDICATE) > buff.size())
+ {
+ std::cerr << "Predicate length field is greater then input buffer: "
+ << (pos + slp::request::SIZE_PREDICATE) << " / "
+ << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
std::copy_n(buff.data() + pos, slp::request::SIZE_PREDICATE,
(uint8_t*)&predicateLen);
predicateLen = endian::from_network(predicateLen);
pos += slp::request::SIZE_PREDICATE;
+ if ((pos + predicateLen) > buff.size())
+ {
+ std::cerr << "Length of Predicate is greater then input buffer: "
+ << (pos + predicateLen) << " / " << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
req.body.srvrqst.predicate.insert(0, (const char*)buff.data() + pos,
predicateLen);
@@ -288,12 +341,25 @@
/* 5) Parse the <SLP SPI> string. */
uint16_t spistrLen;
+ if ((pos + slp::request::SIZE_SLPI) > buff.size())
+ {
+ std::cerr << "SLP SPI length field is greater then input buffer: "
+ << (pos + slp::request::SIZE_SLPI) << " / " << buff.size()
+ << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
std::copy_n(buff.data() + pos, slp::request::SIZE_SLPI,
(uint8_t*)&spistrLen);
spistrLen = endian::from_network(spistrLen);
pos += slp::request::SIZE_SLPI;
+ if ((pos + spistrLen) > buff.size())
+ {
+ std::cerr << "Length of SLP SPI is greater then input buffer: "
+ << (pos + spistrLen) << " / " << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
req.body.srvrqst.spistr.insert(0, (const char*)buff.data() + pos,
spistrLen);
diff --git a/test/slp_parser_test.cpp b/test/slp_parser_test.cpp
index d98bd46..94f999c 100644
--- a/test/slp_parser_test.cpp
+++ b/test/slp_parser_test.cpp
@@ -171,3 +171,140 @@
rc = slp::parser::internal::parseSrvTypeRqst(testData, req);
EXPECT_NE(rc, 0);
}
+
+/* 0 1 2 3
+ 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of <PRList> | <PRList> String \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of <service-type> | <service-type> String \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of <scope-list> | <scope-list> String \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of predicate string | Service Request <predicate> \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of <SLP SPI> string | <SLP SPI> String \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ */
+
+TEST(parseSrvRqst, BasicGoodPath)
+{
+ // Basic buffer with 0 for all lengths
+ slp::buffer testData{0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, /* start of SrvRqst*/
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ slp::Message req;
+ int rc = slp::SUCCESS;
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+
+ EXPECT_EQ(rc, 0);
+}
+
+TEST(parseSrvRqst, GoodPathWithData)
+{
+ // Basic buffer with some valid lengths and data
+ slp::buffer testData{
+ 0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, /* Lang Length */
+ 'L', 'A', 'N', 'G', 0x00, 0x04, /* PRlist length*/
+ 'P', 'R', 'L', 'T', 0x00, 0xB, /* Service type length */
+ 'S', 'E', 'R', 'V', 'I', 'C', 'E',
+ 'T', 'Y', 'P', 'E', 0x00, 0x05, /* Scope length*/
+ 'S', 'C', 'O', 'P', 'E', 0x00, 0x02, /* predicate length */
+ 'P', 'D', 0x00, 0x01, /* SLP SPI length */
+ 'S'};
+ slp::Message req;
+ int rc = slp::SUCCESS;
+ std::tie(rc, req) = slp::parser::internal::parseHeader(testData);
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(req.header.langtag, "LANG");
+ EXPECT_EQ(req.header.langtagLen, 4);
+
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(req.body.srvrqst.prList, "PRLT");
+ EXPECT_EQ(req.body.srvrqst.srvType, "SERVICETYPE");
+ EXPECT_EQ(req.body.srvrqst.scopeList, "SCOPE");
+ EXPECT_EQ(req.body.srvrqst.predicate, "PD");
+ EXPECT_EQ(req.body.srvrqst.spistr, "S");
+}
+
+TEST(parseSrvRqst, GoodPathMatchSlptoolFindSrvs)
+{
+ // This matches what "slptool -u <server> findsrvs service:obmc_console"
+ // sends
+ slp::buffer testData{0x02, 0x01, 0x00, 0x00, 0x35, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0xe5, 0xc2, 0x00, 0x02, /* Lang Length */
+ 'e', 'n', 0x00, 0x00, /* PR list length*/
+ 0x00, 0x14, /* Service length */
+ 's', 'e', 'r', 'v', 'i', 'c', 'e', ':',
+ 'o', 'b', 'm', 'c', '_', 'c', 'o', 'n',
+ 's', 'o', 'l', 'e', 0x00, 0x07, /* Scope length*/
+ 'D', 'E', 'F', 'A', 'U', 'L', 'T', 0x00,
+ 0x00, /* Predicate length */
+ 0x00, 0x00}; /* SLP SPI length*/
+ slp::Message req;
+ int rc = slp::SUCCESS;
+ std::tie(rc, req) = slp::parser::internal::parseHeader(testData);
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(req.header.langtag, "en");
+ EXPECT_EQ(req.header.langtagLen, 2);
+
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(req.body.srvrqst.srvType, "service:obmc_console");
+ EXPECT_EQ(req.body.srvrqst.scopeList, "DEFAULT");
+}
+
+TEST(parseSrvRqst, BadPathSizes)
+{
+ // Basic buffer with invalid PRlist size
+ slp::buffer testData{
+ 0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, /* Lang Length */
+ 'L', 'A', 'N', 'G', 0x00, 0x05, /* PRlist length*/
+ 'P', 'R', 'L', 'T', 0x00, 0xB, /* Service type length */
+ 'S', 'E', 'R', 'V', 'I', 'C', 'E',
+ 'T', 'Y', 'P', 'E', 0x00, 0x05, /* Scope length*/
+ 'S', 'C', 'O', 'P', 'E', 0x00, 0x02, /* predicate length */
+ 'P', 'D', 0x00, 0x01, /* SLP SPI length */
+ 'S'};
+ slp::Message req;
+ int rc = slp::SUCCESS;
+ std::tie(rc, req) = slp::parser::internal::parseHeader(testData);
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(req.header.langtag, "LANG");
+ EXPECT_EQ(req.header.langtagLen, 4);
+
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_NE(rc, 0);
+
+ // Fix PR, make Service type invalid
+ testData[19] = 4;
+ testData[25] = 5;
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_NE(rc, 0);
+
+ // Fix Service type, make Scope invalid
+ testData[25] = 0xB;
+ testData[38] = 10;
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_NE(rc, 0);
+
+ // Fix Scope, make Predicate invalid
+ testData[38] = 5;
+ testData[45] = 100;
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_NE(rc, 0);
+
+ // Fix Predicate, make SLP SPI invalid
+ testData[45] = 2;
+ testData[49] = 5;
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_NE(rc, 0);
+
+ // Now fix SLP SPI and make sure we pass
+ testData[49] = 1;
+ rc = slp::parser::internal::parseSrvRqst(testData, req);
+ EXPECT_EQ(rc, 0);
+}