ADHOC/replicant-frameworks_native
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Disallow reading object data from Parcels with non-object reads

Browse Source 
The check added to each non-object reads adds an overhead. If the
objects (binders and file descriptors) were written to the Parcel in
sequential order then check adds a small O(1) overhead to each read,
plus an O(N) overhead to the first read (to verify the N objects were
added in order).
If the objects were written out of order (as in by jumping around the
Parcel
with setDataPosition and writing Binder, DON'T DO THIS!!) (writing non
objects out of order is fine), the first read is forced to sort the
objects
in the internal bookkeeping. Based on the assumption non sequential
writes
are infrequent and overall Parcels are probably mostly sorted, insertion
sort was used. Worst case sorts will add an O(N^2) overhead to the first
non object read from the Parcel.

Test: run cts -m CtsOsTestCases -t android.os.cts.ParcelTest

Bug: 29833520
Change-Id: I82de8eb5f5eb56f869542d5358e96884c24301b2
(cherry picked from commit c517681c66a1a387be657e0cf06da8d19659dd14)
This commit is contained in:
Michael Wachenschwanz 2017-11-17 18:25:05 -08:00 committed by Tim Schumacher
parent 057269956b
commit d53a5c4aa9
2 changed files with 75 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/binder/Parcel.h
View File

@ -247,6 +247,7 @@ private:
void freeDataNoInit();
void initState();
void scanForFds() const;
status_t validateReadData(size_t len) const;
template<class T>
status_t readAligned(T *pArg) const;
@ -265,6 +266,7 @@ private:
size_t mObjectsSize;
size_t mObjectsCapacity;
mutable size_t mNextObjectHint;
mutable bool mObjectsSorted;
mutable bool mFdsKnown;
mutable bool mHasFds;

73
libs/binder/Parcel.cpp
View File

@ -464,6 +464,7 @@ void Parcel::setDataPosition(size_t pos) const
mDataPos = pos;
mNextObjectHint = 0;
mObjectsSorted = false;
}
status_t Parcel::setDataCapacity(size_t size)
@ -1133,6 +1134,59 @@ void Parcel::remove(size_t /*start*/, size_t /*amt*/)
LOG_ALWAYS_FATAL("Parcel::remove() not yet implemented!");
}
status_t Parcel::validateReadData(size_t upperBound) const
{
// Don't allow non-object reads on object data
if (mObjectsSorted || mObjectsSize <= 1) {
data_sorted:
// Expect to check only against the next object
if (mNextObjectHint < mObjectsSize && upperBound > mObjects[mNextObjectHint]) {
// For some reason the current read position is greater than the next object
// hint. Iterate until we find the right object
size_t nextObject = mNextObjectHint;
do {
if (mDataPos < mObjects[nextObject] + sizeof(flat_binder_object)) {
// Requested info overlaps with an object
ALOGE("Attempt to read from protected data in Parcel %p", this);
return PERMISSION_DENIED;
}
nextObject++;
} while (nextObject < mObjectsSize && upperBound > mObjects[nextObject]);
mNextObjectHint = nextObject;
}
return NO_ERROR;
}
// Quickly determine if mObjects is sorted.
binder_size_t* currObj = mObjects + mObjectsSize - 1;
binder_size_t* prevObj = currObj;
while (currObj > mObjects) {
prevObj--;
if(*prevObj > *currObj) {
goto data_unsorted;
}
currObj--;
}
mObjectsSorted = true;
goto data_sorted;
data_unsorted:
// Insertion Sort mObjects
// Great for mostly sorted lists. If randomly sorted or reverse ordered mObjects become common,
// switch to std::sort(mObjects, mObjects + mObjectsSize);
for (binder_size_t* iter0 = mObjects + 1; iter0 < mObjects + mObjectsSize; iter0++) {
binder_size_t temp = *iter0;
binder_size_t* iter1 = iter0 - 1;
while (iter1 >= mObjects && *iter1 > temp) {
*(iter1 + 1) = *iter1;
iter1--;
}
*(iter1 + 1) = temp;
}
mNextObjectHint = 0;
mObjectsSorted = true;
goto data_sorted;
}
status_t Parcel::read(void* outData, size_t len) const
{
if (len > INT32_MAX) {
@ -1143,6 +1197,10 @@ status_t Parcel::read(void* outData, size_t len) const
if ((mDataPos+pad_size(len)) >= mDataPos && (mDataPos+pad_size(len)) <= mDataSize
&& len <= pad_size(len)) {
if (mObjectsSize > 0) {
status_t err = validateReadData(mDataPos + pad_size(len));
if(err != NO_ERROR) return err;
}
memcpy(outData, mData+mDataPos, len);
mDataPos += pad_size(len);
ALOGV("read Setting data pos of %p to %zu", this, mDataPos);
@ -1161,6 +1219,11 @@ const void* Parcel::readInplace(size_t len) const
if ((mDataPos+pad_size(len)) >= mDataPos && (mDataPos+pad_size(len)) <= mDataSize
&& len <= pad_size(len)) {
if (mObjectsSize > 0) {
status_t err = validateReadData(mDataPos + pad_size(len));
if(err != NO_ERROR) return NULL;
}
const void* data = mData+mDataPos;
mDataPos += pad_size(len);
ALOGV("readInplace Setting data pos of %p to %zu", this, mDataPos);
@ -1174,6 +1237,11 @@ status_t Parcel::readAligned(T *pArg) const {
COMPILE_TIME_ASSERT_FUNCTION_SCOPE(PAD_SIZE_UNSAFE(sizeof(T)) == sizeof(T));
if ((mDataPos+sizeof(T)) <= mDataSize) {
if (mObjectsSize > 0) {
status_t err = validateReadData(mDataPos + sizeof(T));
if(err != NO_ERROR) return err;
}
const void* data = mData+mDataPos;
mDataPos += sizeof(T);
*pArg = *reinterpret_cast<const T*>(data);
@ -1638,6 +1706,7 @@ void Parcel::ipcSetDataReference(const uint8_t* data, size_t dataSize,
mObjects = const_cast<binder_size_t*>(objects);
mObjectsSize = mObjectsCapacity = objectsCount;
mNextObjectHint = 0;
mObjectsSorted = false;
mOwner = relFunc;
mOwnerCookie = relCookie;
for (size_t i = 0; i < mObjectsSize; i++) {
@ -1795,6 +1864,7 @@ status_t Parcel::restartWrite(size_t desired)
mObjects = NULL;
mObjectsSize = mObjectsCapacity = 0;
mNextObjectHint = 0;
mObjectsSorted = false;
mHasFds = false;
mFdsKnown = true;
mAllowFds = true;
@ -1881,6 +1951,7 @@ status_t Parcel::continueWrite(size_t desired)
mDataCapacity = desired;
mObjectsSize = mObjectsCapacity = objectsSize;
mNextObjectHint = 0;
mObjectsSorted = false;
} else if (mData) {
if (objectsSize < mObjectsSize) {
@ -1906,6 +1977,7 @@ status_t Parcel::continueWrite(size_t desired)
}
mObjectsSize = objectsSize;
mNextObjectHint = 0;
mObjectsSorted = false;
}
// We own the data, so we can just do a realloc().
@ -1978,6 +2050,7 @@ void Parcel::initState()
mObjectsSize = 0;
mObjectsCapacity = 0;
mNextObjectHint = 0;
mObjectsSorted = false;
mHasFds = false;
mFdsKnown = true;
mAllowFds = true;