Merge "Protect objects in Parcel::appendFrom" into udc-dev (84a880c4) · Commits · e / os / android_frameworks_native

libs/binder/Parcel.cpp

+28 −12

Original line number	Original line	Diff line number	Diff line
	@@ -493,6 +493,11 @@ status_t Parcel::appendFrom(const Parcel* parcel, size_t offset, size_t len) {
	return BAD_VALUE;		return BAD_VALUE;
	}		}

			// Make sure we aren't appending over objects.
			if (status_t status = validateReadData(mDataPos + len); status != OK) {
			return status;
			}

	if ((mDataPos + len) > mDataCapacity) {		if ((mDataPos + len) > mDataCapacity) {
	// grow data		// grow data
	err = growData(len);		err = growData(len);
	@@ -516,17 +521,13 @@ status_t Parcel::appendFrom(const Parcel* parcel, size_t offset, size_t len) {
	const binder_size_t* objects = otherKernelFields->mObjects;		const binder_size_t* objects = otherKernelFields->mObjects;
	size_t size = otherKernelFields->mObjectsSize;		size_t size = otherKernelFields->mObjectsSize;
	// Count objects in range		// Count objects in range
	int firstIndex = -1, lastIndex = -2;		int numObjects = 0;
	for (int i = 0; i < (int)size; i++) {		for (int i = 0; i < (int)size; i++) {
	size_t off = objects[i];		size_t pos = objects[i];
	if ((off >= offset) && (off + sizeof(flat_binder_object) <= offset + len)) {		if ((pos >= offset) && (pos + sizeof(flat_binder_object) <= offset + len)) {
	if (firstIndex == -1) {		numObjects++;
	firstIndex = i;
	}
	lastIndex = i;
	}		}
	}		}
	int numObjects = lastIndex - firstIndex + 1;
	if (numObjects > 0) {		if (numObjects > 0) {
	const sp<ProcessState> proc(ProcessState::self());		const sp<ProcessState> proc(ProcessState::self());
	// grow objects		// grow objects
	@@ -548,8 +549,12 @@ status_t Parcel::appendFrom(const Parcel* parcel, size_t offset, size_t len) {

	// append and acquire objects		// append and acquire objects
	int idx = kernelFields->mObjectsSize;		int idx = kernelFields->mObjectsSize;
	for (int i = firstIndex; i <= lastIndex; i++) {		for (int i = 0; i < (int)size; i++) {
	size_t off = objects[i] - offset + startPos;		size_t pos = objects[i];
			if (!(pos >= offset) \|\| !(pos + sizeof(flat_binder_object) <= offset + len)) {
			continue;
			}
			size_t off = pos - offset + startPos;
	kernelFields->mObjects[idx++] = off;		kernelFields->mObjects[idx++] = off;
	kernelFields->mObjectsSize++;		kernelFields->mObjectsSize++;

	@@ -568,6 +573,9 @@ status_t Parcel::appendFrom(const Parcel* parcel, size_t offset, size_t len) {
	}		}
	}		}
	}		}
			// Always clear sorted flag. It is tricky to infer if the append
			// result maintains the sort or not.
			kernelFields->mObjectsSorted = false;
	}		}
	#else		#else
	LOG_ALWAYS_FATAL("Binder kernel driver disabled at build time");		LOG_ALWAYS_FATAL("Binder kernel driver disabled at build time");
	@@ -598,7 +606,10 @@ status_t Parcel::appendFrom(const Parcel* parcel, size_t offset, size_t len) {
	const binder_size_t objPos = otherRpcFields->mObjectPositions[i];		const binder_size_t objPos = otherRpcFields->mObjectPositions[i];
	if (offset <= objPos && objPos < offset + len) {		if (offset <= objPos && objPos < offset + len) {
	size_t newDataPos = objPos - offset + startPos;		size_t newDataPos = objPos - offset + startPos;
	rpcFields->mObjectPositions.push_back(newDataPos);		rpcFields->mObjectPositions
			.insert(std::upper_bound(rpcFields->mObjectPositions.begin(),
			rpcFields->mObjectPositions.end(), newDataPos),
			newDataPos);

	mDataPos = newDataPos;		mDataPos = newDataPos;
	int32_t objectType;		int32_t objectType;
	@@ -1473,7 +1484,10 @@ status_t Parcel::writeFileDescriptor(int fd, bool takeOwnership) {
	if (status_t err = writeInt32(rpcFields->mFds->size()); err != OK) {		if (status_t err = writeInt32(rpcFields->mFds->size()); err != OK) {
	return err;		return err;
	}		}
	rpcFields->mObjectPositions.push_back(dataPos);		rpcFields->mObjectPositions
			.insert(std::upper_bound(rpcFields->mObjectPositions.begin(),
			rpcFields->mObjectPositions.end(), dataPos),
			dataPos);
	rpcFields->mFds->push_back(std::move(fdVariant));		rpcFields->mFds->push_back(std::move(fdVariant));
	return OK;		return OK;
	}		}
	@@ -1672,6 +1686,8 @@ restart_write:
	kernelFields->mObjects[kernelFields->mObjectsSize] = mDataPos;		kernelFields->mObjects[kernelFields->mObjectsSize] = mDataPos;
	acquire_object(ProcessState::self(), val, this);		acquire_object(ProcessState::self(), val, this);
	kernelFields->mObjectsSize++;		kernelFields->mObjectsSize++;
			// Clear sorted flag if we aren't appending to the end.
			kernelFields->mObjectsSorted &= mDataPos == mDataSize;
	}		}

	return finishWrite(sizeof(flat_binder_object));		return finishWrite(sizeof(flat_binder_object));

libs/binder/tests/binderParcelUnitTest.cpp

+11 −0

Original line number	Original line	Diff line number	Diff line
	@@ -25,6 +25,7 @@ using android::IBinder;
	using android::IPCThreadState;		using android::IPCThreadState;
	using android::OK;		using android::OK;
	using android::Parcel;		using android::Parcel;
			using android::PERMISSION_DENIED;
	using android::sp;		using android::sp;
	using android::status_t;		using android::status_t;
	using android::String16;		using android::String16;
	@@ -123,6 +124,16 @@ TEST(Parcel, AppendWithBadDataPos) {
	EXPECT_EQ(android::BAD_VALUE, p2.appendFrom(&p1, 0, 8));		EXPECT_EQ(android::BAD_VALUE, p2.appendFrom(&p1, 0, 8));
	}		}

			TEST(Parcel, AppendOverObject) {
			Parcel p1;
			p1.writeDupFileDescriptor(0);
			Parcel p2;
			p2.writeInt32(2);

			p1.setDataPosition(8);
			ASSERT_EQ(PERMISSION_DENIED, p1.appendFrom(&p2, 0, p2.dataSize()));
			}

	// Tests a second operation results in a parcel at the same location as it		// Tests a second operation results in a parcel at the same location as it
	// started.		// started.
	void parcelOpSameLength(const std::function<void(Parcel)>& a, const std::function<void(Parcel)>& b) {		void parcelOpSameLength(const std::function<void(Parcel)>& a, const std::function<void(Parcel)>& b) {