validate m_*Report lengths
Addresses Security concerns:
HIDDevice::Open does not validate minimum sizes for m_*ReportSize, which
could lead to past-end-of-buffer writes when using m_*Report arrays.
HIDDevice::GetAttentionReport does not correctly validate the size of
the m_attnData buffer vs the buf len. This is a past-end-of-buffer read
condition. I don't understand the point of reading bytes-many bytes but
returning *len set to the valid size of bytes in the buffer.
diff --git a/rmidevice/hiddevice.cpp b/rmidevice/hiddevice.cpp
index 07e51cb..6e2a890 100644
--- a/rmidevice/hiddevice.cpp
+++ b/rmidevice/hiddevice.cpp
@@ -240,6 +240,9 @@
else
bytesToRequest = bytesPerRequest;
+ if (m_outputReportSize < HID_RMI4_READ_OUTPUT_COUNT + 2) {
+ return -1;
+ }
m_outputReport[HID_RMI4_REPORT_ID] = RMI_READ_ADDR_REPORT_ID;
m_outputReport[1] = 0; /* old 1 byte read count */
m_outputReport[HID_RMI4_READ_OUTPUT_ADDR] = addr & 0xFF;
@@ -266,6 +269,10 @@
while (bytesReadPerRequest < bytesToRequest) {
rc = GetReport(&reportId);
if (rc > 0 && reportId == RMI_READ_DATA_REPORT_ID) {
+ if (static_cast<ssize_t>(m_inputReportSize) <
+ std::max(HID_RMI4_READ_INPUT_COUNT,
+ HID_RMI4_READ_INPUT_DATA))
+ return -1;
bytesInDataReport = m_readData[HID_RMI4_READ_INPUT_COUNT];
memcpy(buf + bytesReadPerRequest, &m_readData[HID_RMI4_READ_INPUT_DATA],
bytesInDataReport);
@@ -286,6 +293,9 @@
if (!m_deviceOpen)
return -1;
+ if (static_cast<ssize_t>(m_outputReportSize) <
+ HID_RMI4_WRITE_OUTPUT_DATA + len)
+ return -1;
m_outputReport[HID_RMI4_REPORT_ID] = RMI_WRITE_REPORT_ID;
m_outputReport[HID_RMI4_WRITE_OUTPUT_COUNT] = len;
m_outputReport[HID_RMI4_WRITE_OUTPUT_ADDR] = addr & 0xFF;
@@ -353,7 +363,7 @@
unsigned char *buf, unsigned int *len)
{
int rc = 0;
- int bytes = m_inputReportSize;
+ unsigned int bytes = m_inputReportSize;
int reportId;
if (len && m_inputReportSize < *len) {
@@ -367,8 +377,12 @@
rc = GetReport(&reportId, timeout);
if (rc > 0) {
if (reportId == RMI_ATTN_REPORT_ID) {
- if (buf)
+ if (buf) {
+ if (bytes > m_inputReportSize ||
+ m_inputReportSize < HID_RMI4_ATTN_INTERUPT_SOURCES + 1)
+ return -1;
memcpy(buf, m_attnData, bytes);
+ }
if (source_mask & m_attnData[HID_RMI4_ATTN_INTERUPT_SOURCES])
return rc;
}
@@ -389,6 +403,9 @@
if (!m_deviceOpen)
return -1;
+ if (m_inputReportSize < HID_RMI4_REPORT_ID + 1)
+ return -1;
+
for (;;) {
FD_ZERO(&fds);
FD_SET(m_fd, &fds);
@@ -424,10 +441,14 @@
*reportId = m_inputReport[HID_RMI4_REPORT_ID];
if (m_inputReport[HID_RMI4_REPORT_ID] == RMI_ATTN_REPORT_ID) {
- memcpy(m_attnData, m_inputReport, count);
+ if (static_cast<ssize_t>(m_inputReportSize) < count)
+ return -1;
+ memcpy(m_attnData, m_inputReport, count /*offset?*/);
} else if (m_inputReport[HID_RMI4_REPORT_ID] == RMI_READ_DATA_REPORT_ID) {
- memcpy(m_readData, m_inputReport, count);
- m_dataBytesRead = count;
+ if (static_cast<ssize_t>(m_inputReportSize) < count)
+ return -1;
+ memcpy(m_readData, m_inputReport, count /*offset?*/);
+ m_dataBytesRead = count /*offset?*/;
}
return 1;
}
@@ -684,4 +705,4 @@
closedir(devDir);
return ret;
-}
\ No newline at end of file
+}
diff --git a/rmidevice/hiddevice.h b/rmidevice/hiddevice.h
index 97be0e3..05a11fa 100644
--- a/rmidevice/hiddevice.h
+++ b/rmidevice/hiddevice.h
@@ -26,7 +26,11 @@
{
public:
HIDDevice() : RMIDevice(), m_inputReport(NULL), m_outputReport(NULL), m_attnData(NULL),
- m_readData(NULL), m_deviceOpen(false)
+ m_readData(NULL),
+ m_inputReportSize(0),
+ m_outputReportSize(0),
+ m_featureReportSize(0),
+ m_deviceOpen(false)
{}
virtual int Open(const char * filename);
virtual int Read(unsigned short addr, unsigned char *buf,