Only allow a maximum packet size of 255 bytes
This code was written with an assumption that the input/output buffer
sizes would not exceed 255 bytes. The assumption can be seen throughout
the code where it is using a "uint8_t" for size calculations when doing
offsets into the data buffers.
As this application was written for OpenBMC and the service list
provided by OpenBMC is very minimal, supporting a maximum request
buffer of 255 bytes is a fine assumption. Just need to enforce it.
Without this change, very unexpected things can happen with uint8_t
overflow issues when a packet size greater then 255 bytes is passed in
or created internally.
Tested:
- Verified a request with a packet size over 255 bytes is rejected
Change-Id: Icec15fe100d6514a3309fa89e9f79d565de05553
Signed-off-by: Andrew Geissler <geissonator@yahoo.com>
diff --git a/main.cpp b/main.cpp
index 6df25ec..9212820 100644
--- a/main.cpp
+++ b/main.cpp
@@ -25,25 +25,37 @@
return rc;
}
- switch (recvBuff[0])
+ // This code currently assume a maximum of 255 bytes in a receive
+ // or response message. Enforce that here.
+ if (recvBuff.size() > slp::MAX_LEN)
{
- case slp::VERSION_2:
- {
- // Parse the buffer and construct the req object
- std::tie(rc, req) = slp::parser::parseBuffer(recvBuff);
- if (!rc)
- {
- // Passing the req object to handler to serve it
- std::tie(rc, resp) = slp::handler::processRequest(req);
- }
- break;
- }
- default:
- std::cout << "SLP Unsupported Request Version=" << (int)recvBuff[0]
- << "\n";
+ std::cerr << "Message size exceeds maximum allowed: " << recvBuff.size()
+ << " / " << slp::MAX_LEN << std::endl;
- rc = static_cast<uint8_t>(slp::Error::VER_NOT_SUPPORTED);
- break;
+ rc = static_cast<uint8_t>(slp::Error::PARSE_ERROR);
+ }
+ else
+ {
+ switch (recvBuff[0])
+ {
+ case slp::VERSION_2:
+ {
+ // Parse the buffer and construct the req object
+ std::tie(rc, req) = slp::parser::parseBuffer(recvBuff);
+ if (!rc)
+ {
+ // Passing the req object to handler to serve it
+ std::tie(rc, resp) = slp::handler::processRequest(req);
+ }
+ break;
+ }
+ default:
+ std::cout << "SLP Unsupported Request Version="
+ << (int)recvBuff[0] << "\n";
+
+ rc = static_cast<uint8_t>(slp::Error::VER_NOT_SUPPORTED);
+ break;
+ }
}
// if there was error during Parsing of request
diff --git a/slp_message_handler.cpp b/slp_message_handler.cpp
index b159ef7..b0e0cdd 100644
--- a/slp_message_handler.cpp
+++ b/slp_message_handler.cpp
@@ -110,6 +110,20 @@
std::cout << "service=" << service.c_str() << "\n";
+ // See if total response size exceeds our max
+ uint32_t totalLength =
+ buff.size() + /* 14 bytes header + length of langtag */
+ slp::response::SIZE_ERROR + /* 2 byte err code */
+ slp::response::SIZE_SERVICE + /* 2 byte srvtype len */
+ service.length();
+ if (totalLength > slp::MAX_LEN)
+ {
+ std::cerr << "Message response size exceeds maximum allowed: "
+ << totalLength << " / " << slp::MAX_LEN << std::endl;
+ buff.resize(0);
+ return std::make_tuple((int)slp::Error::PARSE_ERROR, buff);
+ }
+
uint8_t length = buff.size() + /* 14 bytes header + length of langtag */
slp::response::SIZE_ERROR + /* 2 byte err code */
slp::response::SIZE_SERVICE + /* 2 byte srvtype len */
@@ -214,6 +228,17 @@
std::string url = svc.name + ':' + svc.type + "//" + addr + ',' +
svc.port;
+ // See if total response size exceeds our max
+ uint32_t totalLength = buff.size() + slp::response::SIZE_URL_ENTRY +
+ url.length();
+ if (totalLength > slp::MAX_LEN)
+ {
+ std::cerr << "Message response size exceeds maximum allowed: "
+ << totalLength << " / " << slp::MAX_LEN << std::endl;
+ buff.resize(0);
+ return std::make_tuple((int)slp::Error::PARSE_ERROR, buff);
+ }
+
buff.resize(buff.size() + slp::response::SIZE_URL_ENTRY + url.length());
uint8_t reserved = 0;
diff --git a/slp_meta.hpp b/slp_meta.hpp
index 538bf9c..aab2f56 100644
--- a/slp_meta.hpp
+++ b/slp_meta.hpp
@@ -12,6 +12,9 @@
/** @brief SLP service lifetime */
constexpr auto LIFETIME = 5;
+/** @brief Largest input or output buffer allowed */
+constexpr size_t MAX_LEN = 255;
+
/** @brief Defines the constants for slp header.
* Size and the offsets.
*/