MTP: add strict bounds checking for all incoming packets
Previously we did not sanity check incoming MTP packets,
which could result in crashes due to reading off the edge of a packet.
Now all MTP packet getter functions return a boolean result
(true for OK, false for reading off the edge of the packet)
and we now return errors for malformed packets.
Bug: 18113092
Change-Id: Ic7623ee96f00652bdfb4f66acb16a93db5a1c105
diff --git a/media/mtp/MtpServer.cpp b/media/mtp/MtpServer.cpp
index aa43967..931a09d 100644
--- a/media/mtp/MtpServer.cpp
+++ b/media/mtp/MtpServer.cpp
@@ -495,6 +495,9 @@
mResponse.setParameter(1, mSessionID);
return MTP_RESPONSE_SESSION_ALREADY_OPEN;
}
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
+
mSessionID = mRequest.getParameter(1);
mSessionOpen = true;
@@ -529,6 +532,9 @@
if (!mSessionOpen)
return MTP_RESPONSE_SESSION_NOT_OPEN;
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
+
MtpStorageID id = mRequest.getParameter(1);
MtpStorage* storage = getStorage(id);
if (!storage)
@@ -550,6 +556,8 @@
MtpResponseCode MtpServer::doGetObjectPropsSupported() {
if (!mSessionOpen)
return MTP_RESPONSE_SESSION_NOT_OPEN;
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectFormat format = mRequest.getParameter(1);
MtpObjectPropertyList* properties = mDatabase->getSupportedObjectProperties(format);
mData.putAUInt16(properties);
@@ -560,6 +568,8 @@
MtpResponseCode MtpServer::doGetObjectHandles() {
if (!mSessionOpen)
return MTP_RESPONSE_SESSION_NOT_OPEN;
+ if (mRequest.getParameterCount() < 3)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpStorageID storageID = mRequest.getParameter(1); // 0xFFFFFFFF for all storage
MtpObjectFormat format = mRequest.getParameter(2); // 0 for all formats
MtpObjectHandle parent = mRequest.getParameter(3); // 0xFFFFFFFF for objects with no parent
@@ -577,6 +587,8 @@
MtpResponseCode MtpServer::doGetNumObjects() {
if (!mSessionOpen)
return MTP_RESPONSE_SESSION_NOT_OPEN;
+ if (mRequest.getParameterCount() < 3)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpStorageID storageID = mRequest.getParameter(1); // 0xFFFFFFFF for all storage
MtpObjectFormat format = mRequest.getParameter(2); // 0 for all formats
MtpObjectHandle parent = mRequest.getParameter(3); // 0xFFFFFFFF for objects with no parent
@@ -599,6 +611,8 @@
return MTP_RESPONSE_SESSION_NOT_OPEN;
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
// FIXME - check for invalid object handle
@@ -617,9 +631,13 @@
return MTP_RESPONSE_SESSION_NOT_OPEN;
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpStorageID handle = mRequest.getParameter(1);
MtpObjectHandleList* references = mData.getAUInt32();
+ if (!references)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpResponseCode result = mDatabase->setObjectReferences(handle, references);
delete references;
return result;
@@ -628,6 +646,8 @@
MtpResponseCode MtpServer::doGetObjectPropValue() {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 2)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
MtpObjectProperty property = mRequest.getParameter(2);
ALOGV("GetObjectPropValue %d %s\n", handle,
@@ -639,6 +659,8 @@
MtpResponseCode MtpServer::doSetObjectPropValue() {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 2)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
MtpObjectProperty property = mRequest.getParameter(2);
ALOGV("SetObjectPropValue %d %s\n", handle,
@@ -648,6 +670,8 @@
}
MtpResponseCode MtpServer::doGetDevicePropValue() {
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpDeviceProperty property = mRequest.getParameter(1);
ALOGV("GetDevicePropValue %s\n",
MtpDebug::getDevicePropCodeName(property));
@@ -656,6 +680,8 @@
}
MtpResponseCode MtpServer::doSetDevicePropValue() {
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpDeviceProperty property = mRequest.getParameter(1);
ALOGV("SetDevicePropValue %s\n",
MtpDebug::getDevicePropCodeName(property));
@@ -664,6 +690,8 @@
}
MtpResponseCode MtpServer::doResetDevicePropValue() {
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpDeviceProperty property = mRequest.getParameter(1);
ALOGV("ResetDevicePropValue %s\n",
MtpDebug::getDevicePropCodeName(property));
@@ -674,6 +702,8 @@
MtpResponseCode MtpServer::doGetObjectPropList() {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 5)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
// use uint32_t so we can support 0xFFFFFFFF
@@ -691,6 +721,8 @@
MtpResponseCode MtpServer::doGetObjectInfo() {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
MtpObjectInfo info(handle);
MtpResponseCode result = mDatabase->getObjectInfo(handle, info);
@@ -732,6 +764,8 @@
MtpResponseCode MtpServer::doGetObject() {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
MtpString pathBuf;
int64_t fileLength;
@@ -765,6 +799,8 @@
}
MtpResponseCode MtpServer::doGetThumb() {
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
size_t thumbSize;
void* thumb = mDatabase->getThumbnail(handle, thumbSize);
@@ -783,6 +819,8 @@
MtpResponseCode MtpServer::doGetPartialObject(MtpOperationCode operation) {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 4)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
uint64_t offset;
uint32_t length;
@@ -832,6 +870,11 @@
MtpResponseCode MtpServer::doSendObjectInfo() {
MtpString path;
+ uint16_t temp16;
+ uint32_t temp32;
+
+ if (mRequest.getParameterCount() < 2)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpStorageID storageID = mRequest.getParameter(1);
MtpStorage* storage = getStorage(storageID);
MtpObjectHandle parent = mRequest.getParameter(2);
@@ -853,25 +896,29 @@
}
// read only the fields we need
- mData.getUInt32(); // storage ID
- MtpObjectFormat format = mData.getUInt16();
- mData.getUInt16(); // protection status
- mSendObjectFileSize = mData.getUInt32();
- mData.getUInt16(); // thumb format
- mData.getUInt32(); // thumb compressed size
- mData.getUInt32(); // thumb pix width
- mData.getUInt32(); // thumb pix height
- mData.getUInt32(); // image pix width
- mData.getUInt32(); // image pix height
- mData.getUInt32(); // image bit depth
- mData.getUInt32(); // parent
- uint16_t associationType = mData.getUInt16();
- uint32_t associationDesc = mData.getUInt32(); // association desc
- mData.getUInt32(); // sequence number
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // storage ID
+ if (!mData.getUInt16(temp16)) return MTP_RESPONSE_INVALID_PARAMETER;
+ MtpObjectFormat format = temp16;
+ if (!mData.getUInt16(temp16)) return MTP_RESPONSE_INVALID_PARAMETER; // protection status
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER;
+ mSendObjectFileSize = temp32;
+ if (!mData.getUInt16(temp16)) return MTP_RESPONSE_INVALID_PARAMETER; // thumb format
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // thumb compressed size
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // thumb pix width
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // thumb pix height
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // image pix width
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // image pix height
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // image bit depth
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // parent
+ if (!mData.getUInt16(temp16)) return MTP_RESPONSE_INVALID_PARAMETER;
+ uint16_t associationType = temp16;
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER;
+ uint32_t associationDesc = temp32; // association desc
+ if (!mData.getUInt32(temp32)) return MTP_RESPONSE_INVALID_PARAMETER; // sequence number
MtpStringBuffer name, created, modified;
- mData.getString(name); // file name
- mData.getString(created); // date created
- mData.getString(modified); // date modified
+ if (!mData.getString(name)) return MTP_RESPONSE_INVALID_PARAMETER; // file name
+ if (!mData.getString(created)) return MTP_RESPONSE_INVALID_PARAMETER; // date created
+ if (!mData.getString(modified)) return MTP_RESPONSE_INVALID_PARAMETER; // date modified
// keywords follow
ALOGV("name: %s format: %04X\n", (const char *)name, format);
@@ -1066,6 +1113,8 @@
MtpResponseCode MtpServer::doDeleteObject() {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 2)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
MtpObjectFormat format = mRequest.getParameter(2);
// FIXME - support deleting all objects if handle is 0xFFFFFFFF
@@ -1087,6 +1136,8 @@
}
MtpResponseCode MtpServer::doGetObjectPropDesc() {
+ if (mRequest.getParameterCount() < 2)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectProperty propCode = mRequest.getParameter(1);
MtpObjectFormat format = mRequest.getParameter(2);
ALOGV("GetObjectPropDesc %s %s\n", MtpDebug::getObjectPropCodeName(propCode),
@@ -1100,6 +1151,8 @@
}
MtpResponseCode MtpServer::doGetDevicePropDesc() {
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpDeviceProperty propCode = mRequest.getParameter(1);
ALOGV("GetDevicePropDesc %s\n", MtpDebug::getDevicePropCodeName(propCode));
MtpProperty* property = mDatabase->getDevicePropertyDesc(propCode);
@@ -1113,6 +1166,8 @@
MtpResponseCode MtpServer::doSendPartialObject() {
if (!hasStorage())
return MTP_RESPONSE_INVALID_OBJECT_HANDLE;
+ if (mRequest.getParameterCount() < 4)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
uint64_t offset = mRequest.getParameter(2);
uint64_t offset2 = mRequest.getParameter(3);
@@ -1180,6 +1235,8 @@
}
MtpResponseCode MtpServer::doTruncateObject() {
+ if (mRequest.getParameterCount() < 3)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
ObjectEdit* edit = getEditObject(handle);
if (!edit) {
@@ -1199,6 +1256,8 @@
}
MtpResponseCode MtpServer::doBeginEditObject() {
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
if (getEditObject(handle)) {
ALOGE("object already open for edit in doBeginEditObject");
@@ -1223,6 +1282,8 @@
}
MtpResponseCode MtpServer::doEndEditObject() {
+ if (mRequest.getParameterCount() < 1)
+ return MTP_RESPONSE_INVALID_PARAMETER;
MtpObjectHandle handle = mRequest.getParameter(1);
ObjectEdit* edit = getEditObject(handle);
if (!edit) {