From 58f5cfa56d5282e69a7580dc4bb97603c409f003 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Wed, 18 Nov 2020 00:32:42 +0000 Subject: libbinder: check null bytes in readString*Inplace This is entirely defensive, since the only real guarantee we have here from these APIs is that a buffer of a given length is available. However, since we write 0's here, presumably to guard against people assuming these are null-terminated strings, we might as well enforce that they are actually null terminated. Bug: 172655291 Test: binderParcelTest (added in newer CL) Change-Id: Ie879112540155f6a93b97aeaf3d41ed8ba4ae79f Merged-In: Ie879112540155f6a93b97aeaf3d41ed8ba4ae79f (cherry picked from commit 51e02b16c397c44ddf81a0736cf6045cd4c44128) --- libs/binder/Parcel.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'libs/binder/Parcel.cpp') diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 9642a87f4e..1f7d27e0e9 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -1869,7 +1869,7 @@ const char* Parcel::readString8Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char* str = (const char*)readInplace(size+1); - if (str != nullptr) { + if (str != nullptr && str[size] == '\0') { return str; } } @@ -1929,7 +1929,7 @@ const char16_t* Parcel::readString16Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char16_t* str = (const char16_t*)readInplace((size+1)*sizeof(char16_t)); - if (str != nullptr) { + if (str != nullptr && str[size] == u'\0') { return str; } } -- cgit v1.2.3-59-g8ed1b From 61d0f84881cfc1bbac513ccd156c56603a48cc90 Mon Sep 17 00:00:00 2001 From: Steven Moreland Date: Fri, 4 Dec 2020 21:13:03 +0000 Subject: libbinder: readString*Inplace SafetyNet (II) SafetyNet logs (this time for failure case, instead of success case). Bug: 172655291 Test: adb logcat -b events | grep snet # exactly one occurance w/ repro (c/p'd from 34af0637666f43ae62040ad1bad76468423feba2) Merged-In: I75ace071693c0a4579ed9477f7b9212a6e27c36d Change-Id: I75ace071693c0a4579ed9477f7b9212a6e27c36d --- libs/binder/Parcel.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) (limited to 'libs/binder/Parcel.cpp') diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index 1f7d27e0e9..b7ad660317 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -1869,8 +1869,11 @@ const char* Parcel::readString8Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char* str = (const char*)readInplace(size+1); - if (str != nullptr && str[size] == '\0') { - return str; + if (str != nullptr) { + if (str[size] == '\0') { + return str; + } + android_errorWriteLog(0x534e4554, "172655291"); } } *outLen = 0; @@ -1929,8 +1932,11 @@ const char16_t* Parcel::readString16Inplace(size_t* outLen) const if (size >= 0 && size < INT32_MAX) { *outLen = size; const char16_t* str = (const char16_t*)readInplace((size+1)*sizeof(char16_t)); - if (str != nullptr && str[size] == u'\0') { - return str; + if (str != nullptr) { + if (str[size] == u'\0') { + return str; + } + android_errorWriteLog(0x534e4554, "172655291"); } } *outLen = 0; -- cgit v1.2.3-59-g8ed1b