From db9a71219f77e03d1f2bf71a0a3562eb8b43de97 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 17 May 2019 13:14:06 -0700 Subject: libbinder: readCString: no ubsan sub-overflow Bug: 131859347 Test: fuzzer Change-Id: I95a0f59684a172925f1eab97ff21e5d14bc79cc8 Merged-In: I95a0f59684a172925f1eab97ff21e5d14bc79cc8 (cherry picked from commit d0d4b584fc294d2c124385644099852918416344) --- libs/binder/Parcel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 2e7edd7a28..8e206f501f 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -2013,8 +2013,8 @@ status_t Parcel::readUtf8FromUtf16(std::unique_ptr* str) const { const char* Parcel::readCString() const { - const size_t avail = mDataSize-mDataPos; - if (avail > 0) { + if (mDataPos < mDataSize) { + const size_t avail = mDataSize-mDataPos; const char* str = reinterpret_cast(mData+mDataPos); // is the string's trailing NUL within the parcel's valid bounds? const char* eos = reinterpret_cast(memchr(str, 0, avail)); -- cgit v1.2.3 From 0038364ef5e07e106ded67ec444158ddb4b02fed Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Mon, 20 May 2019 15:28:13 -0700 Subject: libbinder: Status: check dataPosition sets. Bug: 132650049 Test: fuzzer Change-Id: Id230eae4316a444bc82b416b2049d5a5f589f89a Merged-In: Id230eae4316a444bc82b416b2049d5a5f589f89a (cherry picked from commit 509e0e02730a609c91282625c50a21f731d0f23d) --- libs/binder/Status.cpp | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/libs/binder/Status.cpp b/libs/binder/Status.cpp index a9d5055549..fe0e5924c8 100644 --- a/libs/binder/Status.cpp +++ b/libs/binder/Status.cpp @@ -76,13 +76,23 @@ status_t Status::readFromParcel(const Parcel& parcel) { // Skip over fat response headers. Not used (or propagated) in native code. if (mException == EX_HAS_REPLY_HEADER) { // Note that the header size includes the 4 byte size field. - const int32_t header_start = parcel.dataPosition(); + const size_t header_start = parcel.dataPosition(); + // Get available size before reading more + const size_t header_avail = parcel.dataAvail(); + int32_t header_size; status = parcel.readInt32(&header_size); if (status != OK) { setFromStatusT(status); return status; } + + if (header_size < 0 || static_cast(header_size) > header_avail) { + android_errorWriteLog(0x534e4554, "132650049"); + setFromStatusT(UNKNOWN_ERROR); + return UNKNOWN_ERROR; + } + parcel.setDataPosition(header_start + header_size); // And fat response headers are currently only used when there are no // exceptions, so act like there was no error. @@ -109,19 +119,36 @@ status_t Status::readFromParcel(const Parcel& parcel) { setFromStatusT(status); return status; } + if (remote_stack_trace_header_size < 0 || + static_cast(remote_stack_trace_header_size) > parcel.dataAvail()) { + + android_errorWriteLog(0x534e4554, "132650049"); + setFromStatusT(UNKNOWN_ERROR); + return UNKNOWN_ERROR; + } parcel.setDataPosition(parcel.dataPosition() + remote_stack_trace_header_size); if (mException == EX_SERVICE_SPECIFIC) { status = parcel.readInt32(&mErrorCode); } else if (mException == EX_PARCELABLE) { // Skip over the blob of Parcelable data - const int32_t header_start = parcel.dataPosition(); + const size_t header_start = parcel.dataPosition(); + // Get available size before reading more + const size_t header_avail = parcel.dataAvail(); + int32_t header_size; status = parcel.readInt32(&header_size); if (status != OK) { setFromStatusT(status); return status; } + + if (header_size < 0 || static_cast(header_size) > header_avail) { + android_errorWriteLog(0x534e4554, "132650049"); + setFromStatusT(UNKNOWN_ERROR); + return UNKNOWN_ERROR; + } + parcel.setDataPosition(header_start + header_size); } if (status != OK) { -- cgit v1.2.3