SrvTypeRqst: 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: Id42c8c2f7f87cab07982edabe93f09cd9ef4208a
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
diff --git a/slp_parser.cpp b/slp_parser.cpp
index b79cff2..f99b527 100644
--- a/slp_parser.cpp
+++ b/slp_parser.cpp
@@ -77,6 +77,7 @@
}
else
{
+ req.header.langtagLen = langtagLen;
req.header.langtag.insert(
0, (const char*)buff.data() + slp::header::OFFSET_LANG,
langtagLen);
@@ -117,31 +118,63 @@
/* 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 than 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);
+ pos += slp::request::SIZE_PRLIST;
prListLen = endian::from_network(prListLen);
- req.body.srvtyperqst.prList.insert(
- 0, (const char*)buff.data() + slp::request::OFFSET_PR, prListLen);
+ if ((pos + prListLen) > buff.size())
+ {
+ std::cerr << "Length of PRList is greater than input buffer: "
+ << (slp::request::OFFSET_PR + prListLen) << " / "
+ << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
- uint8_t pos = slp::request::OFFSET_PR + prListLen;
+ req.body.srvtyperqst.prList.insert(0, (const char*)buff.data() + pos,
+ prListLen);
+
+ pos += prListLen;
/* Parse the Naming Authority. */
uint16_t namingAuthLen;
+ if ((pos + slp::request::SIZE_NAMING) > buff.size())
+ {
+ std::cerr << "Naming auth length field is greater than input buffer: "
+ << (pos + slp::request::SIZE_NAMING) << " / " << buff.size()
+ << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
std::copy_n(buff.data() + pos, slp::request::SIZE_NAMING,
(uint8_t*)&namingAuthLen);
pos += slp::request::SIZE_NAMING;
namingAuthLen = endian::from_network(namingAuthLen);
-
if (namingAuthLen == 0 || namingAuthLen == 0xffff)
{
req.body.srvtyperqst.namingAuth = "";
+ // If it's the special 0xffff, treat like 0 size
+ namingAuthLen = 0;
}
else
{
+ if ((pos + namingAuthLen) > buff.size())
+ {
+ std::cerr << "Length of Naming Size is greater than input buffer: "
+ << (pos + namingAuthLen) << " / " << buff.size()
+ << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
req.body.srvtyperqst.namingAuth.insert(
0, (const char*)buff.data() + pos, namingAuthLen);
}
@@ -150,12 +183,25 @@
/* Parse the <scope-list>. */
uint16_t scopeListLen;
+ if ((pos + slp::request::SIZE_SCOPE) > buff.size())
+ {
+ std::cerr << "Length of Scope size is greater than 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);
pos += slp::request::SIZE_SCOPE;
scopeListLen = endian::from_network(scopeListLen);
+ if ((pos + scopeListLen) > buff.size())
+ {
+ std::cerr << "Length of Scope List is greater than input buffer: "
+ << (pos + scopeListLen) << " / " << buff.size() << std::endl;
+ return (int)slp::Error::PARSE_ERROR;
+ }
req.body.srvtyperqst.scopeList.insert(0, (const char*)buff.data() + pos,
scopeListLen);
diff --git a/test/slp_parser_test.cpp b/test/slp_parser_test.cpp
index 338b214..d98bd46 100644
--- a/test/slp_parser_test.cpp
+++ b/test/slp_parser_test.cpp
@@ -66,3 +66,108 @@
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
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | Service Location header (function = SrvTypeRqst = 9) |
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of PRList | <PRList> String \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of Naming Authority | <Naming Authority String> \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ | length of <scope-list> | <scope-list> String \
+ +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+*/
+TEST(parseSrvTypeRqst, BasicGoodPath)
+{
+ // Basic buffer with 0 for all lengths
+ slp::buffer testData{0x00, 0x09, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ slp::Message req;
+ int rc = slp::SUCCESS;
+ rc = slp::parser::internal::parseSrvTypeRqst(testData, req);
+
+ EXPECT_EQ(rc, 0);
+}
+
+TEST(parseSrvTypeRqst, 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, 0x08, /* Naming auth length */
+ 'N', 'A', 'M', 'E', 'A', 'U', 'T',
+ 'H', 0x00, 0x05, /* Scope length*/
+ 'S', 'C', 'O', 'P', 'E'};
+ 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::parseSrvTypeRqst(testData, req);
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(req.body.srvtyperqst.prList, "PRLT");
+ EXPECT_EQ(req.body.srvtyperqst.namingAuth, "NAMEAUTH");
+ EXPECT_EQ(req.body.srvtyperqst.scopeList, "SCOPE");
+}
+
+TEST(parseSrvTypeRqst, GoodPathMatchSlptoolFindSrvTypes)
+{
+ // This matches what "slptool -u <server> findsrvtypes" sends
+ slp::buffer testData{0x02, 0x09, 0x00, 0x00, 0x1d, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x74, 0xe2, 0x00, 0x02, /* Lang Length */
+ 'e', 'n', 0x00, 0x00, /* PRlist length*/
+ 0xff, 0xff, /* Naming auth length */
+ 0x00, 0x07, /* Scope length*/
+ 'D', 'E', 'F', 'A', 'U', 'L', 'T'};
+ 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::parseSrvTypeRqst(testData, req);
+ EXPECT_EQ(rc, 0);
+ EXPECT_EQ(req.body.srvtyperqst.namingAuth, "");
+ EXPECT_EQ(req.body.srvtyperqst.scopeList, "DEFAULT");
+}
+
+TEST(parseSrvTypeRqst, BadPathSizes)
+{
+ // 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, 0x05, /* PRlist length*/
+ 'P', 'R', 'L', 'T', 0x00, 0x08, /* Naming auth length */
+ 'N', 'A', 'M', 'E', 'A', 'U', 'T',
+ 'H', 0x00, 0x05, /* Scope length*/
+ 'S', 'C', 'O', 'P', 'E'};
+ 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::parseSrvTypeRqst(testData, req);
+ EXPECT_NE(rc, 0);
+
+ // Fix PR, make Name invalid
+ testData[19] = 4;
+ testData[25] = 5;
+ rc = slp::parser::internal::parseSrvTypeRqst(testData, req);
+ EXPECT_NE(rc, 0);
+
+ // Fix Name, make Scope invalid
+ testData[25] = 8;
+ testData[35] = 10;
+ rc = slp::parser::internal::parseSrvTypeRqst(testData, req);
+ EXPECT_NE(rc, 0);
+}